Skip to content

Conversation

@promag
Copy link
Contributor

@promag promag commented Nov 1, 2015

No description provided.

@laanwj
Copy link
Member

laanwj commented Nov 2, 2015

Can you explain what is the issue that this is fixing, and how it fixes it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@laanwj This was wrong.

@laanwj
Copy link
Member

laanwj commented Nov 2, 2015

Agree re: Initialize/Shutdown. As Initialize is always immediately called after the constructor and Shutdown before the destructor it makes little sense to require these to be called explicitly (and have it possible for the object to be in 'created but uninitialized' state.

@promag
Copy link
Contributor Author

promag commented Nov 2, 2015

@laanwj 👍

@jonasschnelli
Copy link
Contributor

Codereview ACK.
Makes sense to use the destructor instead init.cpp pollution. This also fixes a possible tiny-shutdown-only memory leak in case of something went wrong during initialization.

@laanwj
Copy link
Member

laanwj commented Nov 4, 2015

utACK.

Please do include more information in your commit message. With an empty description, someone browsing git in the future will not be able to understand the reasoning behind your change.

Moves the call Initialize() from init.cpp to CreateWithArguments() and handles the
return value. Moves the call Shutdown() from init.cpp to destructor.
Changes Initialize() and Shutdown() to protected members.
@promag promag force-pushed the bugfix/zmq-initialization-shutdown branch from 7695f9a to de0499d Compare November 4, 2015 10:53
@promag
Copy link
Contributor Author

promag commented Nov 4, 2015

@laanwj 👌

@laanwj laanwj merged commit de0499d into bitcoin:master Nov 4, 2015
laanwj added a commit that referenced this pull request Nov 4, 2015
de0499d Fix ZMQ Notification initialization and shutdown (João Barbosa)
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Nov 27, 2015
Moves the call Initialize() from init.cpp to CreateWithArguments() and handles the
return value. Moves the call Shutdown() from init.cpp to destructor.
Changes Initialize() and Shutdown() to protected members.

Github-Pull: bitcoin#6927
Rebased-From: de0499d
zkbot added a commit to zcash/zcash that referenced this pull request Feb 9, 2017
zkbot added a commit to zcash/zcash that referenced this pull request Feb 9, 2017
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants