-
Notifications
You must be signed in to change notification settings - Fork 37.7k
net: only delete CConnman if it's been created #8715
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
Merged
Merged
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
Concept ACK, but wouldn't it be easier to invoke |
As discussed on IRC, yes. This is over-engineered as-is, for future threading scenarios that may not ever happen. I'll make that change. |
In the case of (for example) an already-running bitcoind, the shutdown sequence begins before CConnman has been created, leading to a null-pointer dereference when g_connman->Stop() is called. Instead, Just let the CConnman dtor take care of stopping.
f7b298b
to
36fa01f
Compare
Replaced with the simpler version. |
utACK 36fa01f |
laanwj
added a commit
that referenced
this pull request
Sep 14, 2016
36fa01f net: only delete CConnman if it's been created (Cory Fields)
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.
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 fixes a possible shutdown crash. Thanks to @morcos for reporting.
In the case of (for example) an already-running bitcoind, the shutdown sequence begins before CConnman has been created, leading to a null-pointer dereference when g_connman->Stop() is called.