-
Notifications
You must be signed in to change notification settings - Fork 20.1k
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
core: fix transaction event asynchronicity #16843
Conversation
…lay, Raft minter with NewTxsEvent
@karalabe did you mean by pool.txFeed.Send(NewTxsEvent{promoted}) , whoever subscribed NewTxsEvent would block the Send itself till it's process done ? Literally I find "// TrySend attempts to send x on the channel v but will not block." at reflect/value.go TrySend():1733 noting it's not a blocked sending when invoking feed.send() |
seems this change make goroutine leak as reported in #17450 (comment) |
@hadv I got you , but I'm in a mess slightly here by karalabe saying that "If a subsystem is however subscribed to transaction events and at the same time waiting for the transaction pool lock, this synchronicity will lock the process up". It's a non-block invoke by pool.txFeed.Send() , because msgs sent by cases[i].Chan.TrySend(rvalue) and the feeder won't wait a slow consumer , instead return false , then it's quoted in loop till succeeded .
Extremely , if consumers subscribed the Feed go to ask for pool again , they just block themselves , I mean , it shouldn't occur a dead lock . |
I think that the return false will make goroutine leak because it never reach the code to unlock the sync |
Fixes #16840.
#16720 reworked transaction event raises and accidentally removed a goroutine when the transaction pool promoted a transaction (old vs new). This unfortunately means that the transaction pool lock is kept held while the event is being processed. If a subsystem is however subscribed to transaction events and at the same time waiting for the transaction pool lock, this synchronicity will lock the process up. This PR fixes the oversight.