Skip to content

Payment service: fixed race condition in PaymentProcessor #1144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 11, 2021

Conversation

Wiezzel
Copy link

@Wiezzel Wiezzel commented Mar 10, 2021

If NotifyPayment got called during processing of SchedulePayment (which happened when running debit_note_flow example with dummy driver) it would fail because the payment order had not been saved in the database yet.

To avoid this race condition (and possibly others) PaymentProcessor has been wrapped in a mutex. This allowed to remove the mutex from DriverRegistry. Also, sending SendPayment message needs to be done via Arbiter::spawn now to avoid deadlock in case it is sent to the same node.


Testing instructions: Run debit_note_flow with dummy driver. (A few times preferably)

If `NotifyPayment` got called during processing of `SchedulePayment`
(which happened when running `debit_note_flow` example with dummy
driver) it would fail because the payment order had not been saved in
the database yet.

To avoid this race condition (and possibly others) PaymentProcessor
has been wrapped in a mutex. This allowed to remove the mutex from
DriverRegistry. Also, sending `SendPayment` message needs to be done
via `Arbiter::spawn` now to avoid deadlock in case it is sent to the
same node.

Signed-off-by: Adam Wierzbicki <[email protected]>
@Wiezzel Wiezzel requested review from pnowosie, maaktweluit and a team March 10, 2021 15:40
@Wiezzel Wiezzel self-assigned this Mar 10, 2021
Copy link
Contributor

@pnowosie pnowosie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work!
...and dummy driver for the win 🎉

... debit_note_flow]  👍🏻 Example completed successfully ❤️

Copy link
Contributor

@maaktweluit maaktweluit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Tested the flows with both drivers ( erc20 is currently broken )

@pnowosie pnowosie merged commit a332990 into master Mar 11, 2021
@pnowosie pnowosie deleted the wiezzel/payment-processor-lock branch March 11, 2021 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants