Remove more boost threads #12381
Merged
Remove more boost threads #12381
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
|
Very nice! Concept ACK! |
laanwj
reviewed
Feb 8, 2018
src/qt/optionsmodel.cpp
Outdated
Member
There was a problem hiding this comment.
Probably call InterruptMapPort here too, before Stop, for consistency.
Edit: Oh, you have Stop call Interrupt? That's different from other [Start|Stop|Interrupt] constructions. As you probably know, the idea behind having a seperate interrupt is to have a two-stage (parallel) shutdown process, in which the interrupt can ask the subsystem to shut down, then Stop waits for it to really have stopped. Calling Stop without Interrupt could generally cause it to wait forever.
Member
There was a problem hiding this comment.
Sure - though if interrupt is idempotent there's no problem in calling it in Stop too.
theuni
force-pushed
the
boost-threads-again
branch
from
3c7b673 to
004f999
Compare
Member
Author
|
Updated for @laanwj's suggestion, though I didn't see the second comment until I'd already pushed. Diff from before is trivial: diff --git a/src/net.cpp b/src/net.cpp
index cd9a5e1..2019146 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1562,3 +1562,2 @@ void StopMapPort()
if(g_upnp_thread.joinable()) {
- InterruptMapPort();
g_upnp_thread.join();
diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp
index ee8ba65..909be1c 100644
--- a/src/qt/optionsmodel.cpp
+++ b/src/qt/optionsmodel.cpp
@@ -320,2 +320,3 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in
} else {
+ InterruptMapPort();
StopMapPort(); |
Member
|
reut-ACK 004f999, thanks |
Empact
reviewed
Feb 8, 2018
| static std::unique_ptr<boost::thread> upnp_thread; | ||
| if (!g_upnp_thread.joinable()) { | ||
| assert(!g_upnp_interrupt); | ||
| g_upnp_thread = std::thread((std::bind(&TraceThread<void (*)()>, "upnp", &ThreadMapPort))); |
Empact
reviewed
Feb 8, 2018
| } | ||
| } | ||
|
|
||
| #else |
Contributor
There was a problem hiding this comment.
Maybe worth commenting that this is the USE_UPNP disabled case?
Member
Author
|
Since there's already an utACK, I'd prefer to leave @Empact's nits alone, but I'll be sure to include them if more changes are needed. |
Contributor
|
utACK 004f999 |
laanwj
added a commit
that referenced
this pull request
Feb 12, 2018
004f999 boost: drop boost threads for [alert|block|wallet]notify (Cory Fields) 0827267 boost: drop boost threads from torcontrol (Cory Fields) ba91724 boost: remove useless threadGroup parameter from Discover (Cory Fields) f26866b boost: drop boost threads for upnp (Cory Fields) Pull request description: This doesn't completely get rid of boost::thread, but this batch should be easy to review, and leaves us with only threadGroup (scheduler + scriptcheck) remaining. Note to reviewers: The upnp diff changes a bunch of whitespace, it's much more clear with 'git diff -w' Tree-SHA512: 5a356798d0785f93ed143d1f0afafe890bc82f0d470bc969473da2d2aa78bcb9b096f7ba11b92564d546fb447d4bd0d347e7842994ea0170aafd53fda7e0a66e
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jun 13, 2020
004f999 boost: drop boost threads for [alert|block|wallet]notify (Cory Fields) 0827267 boost: drop boost threads from torcontrol (Cory Fields) ba91724 boost: remove useless threadGroup parameter from Discover (Cory Fields) f26866b boost: drop boost threads for upnp (Cory Fields) Pull request description: This doesn't completely get rid of boost::thread, but this batch should be easy to review, and leaves us with only threadGroup (scheduler + scriptcheck) remaining. Note to reviewers: The upnp diff changes a bunch of whitespace, it's much more clear with 'git diff -w' Tree-SHA512: 5a356798d0785f93ed143d1f0afafe890bc82f0d470bc969473da2d2aa78bcb9b096f7ba11b92564d546fb447d4bd0d347e7842994ea0170aafd53fda7e0a66e
UdjinM6
added a commit
to dashpay/dash
that referenced
this pull request
Jun 13, 2020
* Merge bitcoin#12381: Remove more boost threads 004f999 boost: drop boost threads for [alert|block|wallet]notify (Cory Fields) 0827267 boost: drop boost threads from torcontrol (Cory Fields) ba91724 boost: remove useless threadGroup parameter from Discover (Cory Fields) f26866b boost: drop boost threads for upnp (Cory Fields) Pull request description: This doesn't completely get rid of boost::thread, but this batch should be easy to review, and leaves us with only threadGroup (scheduler + scriptcheck) remaining. Note to reviewers: The upnp diff changes a bunch of whitespace, it's much more clear with 'git diff -w' Tree-SHA512: 5a356798d0785f93ed143d1f0afafe890bc82f0d470bc969473da2d2aa78bcb9b096f7ba11b92564d546fb447d4bd0d347e7842994ea0170aafd53fda7e0a66e * fix using std::thread Signed-off-by: pasta <[email protected]> * Switch to std::thread in NotifyTransactionLock * Move StopTorControl call from Shutdown to PrepareShutdown Co-authored-by: Wladimir J. van der Laan <[email protected]> Co-authored-by: UdjinM6 <[email protected]>
random-zebra
added a commit
to PIVX-Project/PIVX
that referenced
this pull request
Aug 27, 2020
e733939 [Cleanup] Remove CConnman::Copy(Release)NodeVector, now unused (random-zebra) b09da57 [Refactor] Proper CConnman encapsulation of mnsync (random-zebra) 219061e [Refactor] Decouple SyncWithNode from CMasternodeSync::Process() (random-zebra) e1f3620 [Refactor] Proper CConnman encapsulation of proposals / votes sync (random-zebra) f38c9f9 miner: don't try to access connman if it doesn't exist (Fuzzbawls) 92ab619 Address feedback (Fuzzbawls) 9d5346a Do not set an addr time penalty when a peer advertises itself. (Gregory Maxwell) fe12ff9 net: No longer send local address in addrMe (Wladimir J. van der Laan) 14d6917 net: misc header cleanups (Cory Fields) 34ba0de net: make proxy receives interruptible (Cory Fields) 1bd97ef net: make net processing interruptable (Fuzzbawls) e24b4cc net: make net interruptible (Cory Fields) 814d0de net: add CThreadInterrupt and InterruptibleSleep (Cory Fields) 1443f2a net: a few small cleanups before replacing boost threads (Cory Fields) 58eabe4 net: move MAX_FEELER_CONNECTIONS into connman (Cory Fields) 874baba Convert ForEachNode* functions to take a templated function argument rather than a std::function to eliminate std::function overhead (Jeremy Rubin) 9485ab0 Made the ForEachNode* functions in src/net.cpp more pragmatic and self documenting (Jeremy Rubin) c1e59ad minor net cleanups (Fuzzbawls) 07ae004 net: move vNodesDisconnected into CConnman (Cory Fields) 276c946 net: add nSendBufferMaxSize/nReceiveFloodSize to CConnection::Options (Cory Fields) 22a9aff net: Introduce CConnection::Options to avoid passing so many params (Cory Fields) e4891bf net: Drop StartNode/StopNode and use CConnman directly (Cory Fields) 431575c net: pass CClientUIInterface into CConnman (Cory Fields) 48de47e net: Pass best block known height into CConnman (Cory Fields) 15eed91 net: move max/max-outbound to CConnman (Cory Fields) 2bf0921 net: move semOutbound to CConnman (Cory Fields) 481929f net: move nLocalServices/nRelevantServices to CConnman (Cory Fields) bcee6ae net: move SendBufferSize/ReceiveFloodSize to CConnman (Cory Fields) 6865469 net: move send/recv statistics to CConnman (Cory Fields) 1cec418 net: SocketSendData returns written size (Cory Fields) 2bb9dfa net: move messageHandlerCondition to CConnman (Cory Fields) 9c5a0df net: move nLocalHostNonce to CConnman (Cory Fields) a1394ef net: move nLastNodeId to CConnman (Cory Fields) 3804c29 net: move whitelist functions into CConnman (Cory Fields) dbde9be net: create generic functor accessors and move vNodes to CConnman (Cory Fields) 2e02467 net: Add most functions needed for vNodes to CConnman (Cory Fields) 5667e61 net: move added node functions to CConnman (Cory Fields) 37487ed net: Add oneshot functions to CConnman (Cory Fields) facf878 net: move ban and addrman functions into CConnman (Cory Fields) 091aaf2 net: handle nodesignals in CConnman (Cory Fields) 1e9fa0f net: move OpenNetworkConnection into CConnman (Cory Fields) 573200f net: Move socket binding into CConnman (Cory Fields) 7762b97 net: Pass CConnection to wallet rather than using the global (Fuzzbawls) 00591b8 net: Pass CConnman around as needed (Cory Fields) 2cd3d39 net: Add rpc error for missing/disabled p2p functionality (Cory Fields) f08e316 net: Create CConnman to encapsulate p2p connections (Cory Fields) 66337dc net: move CBanDB and CAddrDB out of net.h/cpp (Fuzzbawls) 10969f6 gui: add NodeID to the peer table (Cory Fields) 58044ac Fix some locks (Pieter Wuille) f9f8926 Do not shadow variables in networking code (Pavel Janík) Pull request description: This is the finalization of the upstream PRs being tracked in #1374 to update much of our networking code to more closely follow upstream improvements. The following PRs are included here: - bitcoin#8466 Do not shadow variables in networking code - bitcoin#8606 Fix some locks - bitcoin#8085 Begin encapsulation - bitcoin#9289 drop boost::thread_group - bitcoin#8740 No longer send local address in addrMe - bitcoin#8661 Do not set an addr time penalty when a peer advertises itself Additionally, during conflict resolution and backporting of 8085, the following additional upstream PR was included: - bitcoin#8715 only delete CConnman if it's been created Still TODO in future PRs: - bitcoin#9037 - bitcoin#9441 - bitcoin#9609 - bitcoin#9626 - bitcoin#9708 - bitcoin#12381 - bitcoin#18584 ACKs for top commit: furszy: same here, code ACK e733939. Nice work ☕ . furszy: ACK e733939 . random-zebra: ACK e733939 and merging... Tree-SHA512: 0fc3cca76d9ddc13f75fc9d48c48d215e6c0e0381377c0318436176a0e0ead73b511e0061a4b63f7052874730b6da8b0ffc2c94cba034bcc39aad4212b69ee22
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 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.
This doesn't completely get rid of boost::thread, but this batch should be easy to review, and leaves us with only threadGroup (scheduler + scriptcheck) remaining.
Note to reviewers: The upnp diff changes a bunch of whitespace, it's much more clear with 'git diff -w'