fix: tor: Call event_base_loopbreak from the event's callback #16405
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
To backport. |
Member
|
Yes, this seems the most straightforward solution. |
promag
added a commit
to promag/bitcoin
that referenced
this pull request
Jul 17, 2019
Github-Pull: bitcoin#16405 Rebased-From: a981e74
fanquake
approved these changes
Jul 18, 2019
Member
fanquake
left a comment
fanquake
left a comment
There was a problem hiding this comment.
ACK a981e74
Tested that the change fixes the hanging behavior at shutdown using the above command.
In regards to the code. If I understand correctly we are calling event_base_once to trigger a one-time event, and by passing EV_TIMEOUT as events, and nullptr to timeout, this causes the callback to be executed immediately. The callback calls event_base_loopbreak and that will:
Abort the active event_base_loop() immediately."
This is required because we haven't yet called event_base_dispatch, which would be done just after this code.
Contributor
Author
to be queued and called asap by the event base. |
laanwj
added a commit
that referenced
this pull request
Jul 18, 2019
…llback a981e74 fix: tor: Call event_base_loopbreak from the event's callback (João Barbosa) Pull request description: Calling `event_base_loopbreak` before `event_base_dispatch` has no effect. Fix this by calling `event_base_loopbreak` from the event's callback. From the [documentation](http://www.wangafu.net/~nickm/libevent-2.0/doxygen/html/event_8h.html#a07a7599e478e4031fa8cf52e26d8aa1e): > event_base_loop() will abort the loop after the next event is completed; event_base_loopbreak() is typically invoked from this event's callback. This behavior is analogous to the "break;" statement. This can be tested by running the following with and without this change: ```sh bitcoind -- -regtest -proxy=127.0.0.1:9050 -listen=1 -bind=127.0.0.1 -whitebind=127.0.0.1:0 ``` Fixes #16376. ACKs for top commit: laanwj: code review ACK a981e74 fanquake: ACK a981e74 Tree-SHA512: 328fe71366404d5be8177d7081d5b4868cee73412df631a1865d24fb1c153427210762738109e06b737f037f4c68966812fba041831bb9e8129861f19ce61a63
fanquake
added a commit
that referenced
this pull request
Jul 18, 2019
…t's callback b2711b9 fix: tor: Call event_base_loopbreak from the event's callback (João Barbosa) Pull request description: Github-Pull: #16405 Rebased-From: a981e74 ACKs for top commit: laanwj: ACK b2711b9, code change is the same as for master fanquake: ACK b2711b9 Tree-SHA512: 9f225e505c0241be422ed897f56aef6ebad57e15d3bfe5154c7fe4f874df342e0df287871cd737eb777d0f45865a6d129cd5d1a4c036ea0a4e5d4f36520ab174
ghost
mentioned this pull request
HashUnlimited
pushed a commit
to HashUnlimited/chaincoin
that referenced
this pull request
Aug 23, 2019
Github-Pull: bitcoin#16405 Rebased-From: a981e74
Bushstar
pushed a commit
to Bushstar/omnicore
that referenced
this pull request
Aug 24, 2019
Github-Pull: bitcoin#16405 Rebased-From: a981e74
jasonbcox
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Oct 12, 2020
Summary: PR description: > Calling event_base_loopbreak before event_base_dispatch has no effect. Fix this by calling event_base_loopbreak from the event's callback. From the documentation: > >> event_base_loop() will abort the loop after the next event is completed; event_base_loopbreak() is typically invoked from this event's callback. This behavior is analogous to the "break;" statement. The issue was: > When I run > ./bitcoind -regtest -whitebind=bla > I expect a clean shutdown with Error: Cannot resolve -whitebind address: 'bla'. > However sometimes my node doesn't stop and freezes (shutdown never finishes). Backport of Core [[bitcoin/bitcoin#16405 | PR16405]] Test Plan: `ninja all check-all` `./src/bitcoind -regtest -proxy=127.0.0.1:9050 -listen=1 -bind=127.0.0.1 -whitebind=127.0.0.1:0` After this patch, bitcoind shuts down properly after `Error: Cannot resolve -whitebind address: ...` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D7884
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jul 1, 2021
…nt's callback a981e74 fix: tor: Call event_base_loopbreak from the event's callback (João Barbosa) Pull request description: Calling `event_base_loopbreak` before `event_base_dispatch` has no effect. Fix this by calling `event_base_loopbreak` from the event's callback. From the [documentation](http://www.wangafu.net/~nickm/libevent-2.0/doxygen/html/event_8h.html#a07a7599e478e4031fa8cf52e26d8aa1e): > event_base_loop() will abort the loop after the next event is completed; event_base_loopbreak() is typically invoked from this event's callback. This behavior is analogous to the "break;" statement. This can be tested by running the following with and without this change: ```sh bitcoind -- -regtest -proxy=127.0.0.1:9050 -listen=1 -bind=127.0.0.1 -whitebind=127.0.0.1:0 ``` Fixes bitcoin#16376. ACKs for top commit: laanwj: code review ACK a981e74 fanquake: ACK a981e74 Tree-SHA512: 328fe71366404d5be8177d7081d5b4868cee73412df631a1865d24fb1c153427210762738109e06b737f037f4c68966812fba041831bb9e8129861f19ce61a63
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jul 13, 2021
…nt's callback a981e74 fix: tor: Call event_base_loopbreak from the event's callback (João Barbosa) Pull request description: Calling `event_base_loopbreak` before `event_base_dispatch` has no effect. Fix this by calling `event_base_loopbreak` from the event's callback. From the [documentation](http://www.wangafu.net/~nickm/libevent-2.0/doxygen/html/event_8h.html#a07a7599e478e4031fa8cf52e26d8aa1e): > event_base_loop() will abort the loop after the next event is completed; event_base_loopbreak() is typically invoked from this event's callback. This behavior is analogous to the "break;" statement. This can be tested by running the following with and without this change: ```sh bitcoind -- -regtest -proxy=127.0.0.1:9050 -listen=1 -bind=127.0.0.1 -whitebind=127.0.0.1:0 ``` Fixes bitcoin#16376. ACKs for top commit: laanwj: code review ACK a981e74 fanquake: ACK a981e74 Tree-SHA512: 328fe71366404d5be8177d7081d5b4868cee73412df631a1865d24fb1c153427210762738109e06b737f037f4c68966812fba041831bb9e8129861f19ce61a63
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Calling
event_base_loopbreakbeforeevent_base_dispatchhas no effect. Fix this by callingevent_base_loopbreakfrom the event's callback. From the documentation:This can be tested by running the following with and without this change:
Fixes #16376.