search

OpenBazaar / multiwallet

API based multi-cryptocurrency wallet
MIT License
75 stars 41 forks source link

Rotation Manager data race fix #89

Closed andreygrehov closed 5 years ago

andreygrehov commented 5 years ago

r.started needs to be protected from race condition.

placer14 commented 5 years ago

Thanks for submitting this. We don't have good tests around proving clean startup/shutdown. What steps did you take to prove this change is an improvement so I can independently verify (and possibly include tests for)?

andreygrehov commented 5 years ago

@placer14 i only used Go's data race detector. Updated the PR to address @cpacia's concern.

cpacia [6:22 PM] I think acquireCurrent is going to try to acquire the same lock and end up blocking in this case. We could add another mutex for the started bool. In general this part of the app really needs to be rewritten because it's too complex and source of a good deal of bugs.

andreygrehov commented 5 years ago

Closing it out for now. Need more tests.