Skip to content
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

net: only delete CConnman if it's been created #8715

Merged
merged 1 commit into from Sep 14, 2016

Conversation

@theuni
Copy link
Member

commented Sep 13, 2016

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.

@jonasschnelli jonasschnelli added the P2P label Sep 13, 2016

@sipa

This comment has been minimized.

Copy link
Member

commented Sep 13, 2016

Concept ACK, but wouldn't it be easier to invoke CConnman::Stop() from CConnman::~CConnman, and then call g_connman.reset() at the time you'd stop?

@theuni

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2016

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.

net: only delete CConnman if it's been created
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.

@theuni theuni force-pushed the theuni:fix-connman-shutdown branch to 36fa01f Sep 14, 2016

@theuni

This comment has been minimized.

Copy link
Member Author

commented Sep 14, 2016

Replaced with the simpler version.

@laanwj

This comment has been minimized.

Copy link
Member

commented Sep 14, 2016

utACK 36fa01f

@laanwj laanwj merged commit 36fa01f into bitcoin:master Sep 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Sep 14, 2016
Merge #8715: net: only delete CConnman if it's been created
36fa01f net: only delete CConnman if it's been created (Cory Fields)
@dagurval dagurval referenced this pull request May 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.