Skip to content

do not hold lock on whole publish#53

Open
Snawoot wants to merge 1 commit intoasaskevich:masterfrom
mysteriumnetwork:relax_locking
Open

do not hold lock on whole publish#53
Snawoot wants to merge 1 commit intoasaskevich:masterfrom
mysteriumnetwork:relax_locking

Conversation

@Snawoot
Copy link

@Snawoot Snawoot commented Apr 14, 2022

Fix #35
Fix #52

This PR does not holds lock during whole publish process. It saves from deadlocks when callbacks also trying to publish something or manage subscription on the bus.

Before this publish worked with copy of active handlers anyway, so now it's safe to work with bus because it uses locks only to manage internal state, not causing deadlock in subsequent calls from handlers.

There is a slight change in the handler object: once boolean flag replaced with pointer to sync.Once. This achieves following goals:

  • Use separate synchronization to ensure handler invoked only once.
  • Uses pointer to allow sync.Once to be nullable and skip it when not needed.
  • Uses uniqueness property of pointers in handlers slice to reliably find own record for removal.

@Tochemey
Copy link

Hello @Snawoot and @millken it seems the PR is not merged. Any reason why?

@Snawoot
Copy link
Author

Snawoot commented Dec 24, 2022

@Tochemey Because only @asaskevich can merge.

@Tochemey
Copy link

Hello @asaskevich any reason this PR is not yet merged. Also do you advice to still use this library in production because I have not seen any update to it.?

@Tochemey Because only @asaskevich can merge.

Thank you so much @Snawoot

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.

Lock aquired when publish an event from subscriber. Potential deadlock due to calling callbacks while holding a lock

4 participants