Fixes #1762 segfault when miner is interrupted. #1778

Merged
merged 1 commit into from Nov 6, 2016

Conversation

Projects
None yet
4 participants
@bitcartel
Contributor

bitcartel commented Nov 4, 2016

Closes #1762

@bitcartel bitcartel added this to the 1.0.2 milestone Nov 4, 2016

@bitcartel bitcartel self-assigned this Nov 4, 2016

@bitcartel

This comment has been minimized.

Show comment
Hide comment
@bitcartel

bitcartel Nov 4, 2016

Contributor

Performed testing of this PR on mainnet by running zcashd in gdb with and without the patch, using zcash-cli to enable and disable the miner, and then wait for a new block to arrive and either crash (without patch) or continue as normal (with patch).

Contributor

bitcartel commented Nov 4, 2016

Performed testing of this PR on mainnet by running zcashd in gdb with and without the patch, using zcash-cli to enable and disable the miner, and then wait for a new block to arrive and either crash (without patch) or continue as normal (with patch).

src/miner.cpp
@@ -648,12 +648,10 @@ void static BitcoinMiner(CWallet *pwallet)
catch (const boost::thread_interrupted&)
{
LogPrintf("ZcashMiner terminated\n");
- throw;

This comment has been minimized.

@str4d

str4d Nov 4, 2016

Contributor

AIUI this is technically wrong; Boost requires that you re-throw boost::thread_interrupted so that higher-level code can catch it.

@str4d

str4d Nov 4, 2016

Contributor

AIUI this is technically wrong; Boost requires that you re-throw boost::thread_interrupted so that higher-level code can catch it.

Fixes #1762 segfault when miner is interrupted.
Running ./zcash-cli setgenerate false would result in a segfault.
The miner thread's boost::signals2::connection was not disconnected
when the miner thread was interrupted and shutdown.  Subsequently, when
a new block arrived, the UpdateTip callback would still be invoked on
a now invalid object, resulting in a segfault.
@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Nov 4, 2016

Contributor

utACK

Contributor

str4d commented Nov 4, 2016

utACK

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Nov 5, 2016

Contributor

utACK

Contributor

daira commented Nov 5, 2016

utACK

@str4d str4d removed the review needed label Nov 5, 2016

@str4d

This comment has been minimized.

Show comment
Hide comment
@str4d

str4d Nov 5, 2016

Contributor

@zkbot r+

Contributor

str4d commented Nov 5, 2016

@zkbot r+

@zkbot

This comment has been minimized.

Show comment
Hide comment
@zkbot

zkbot Nov 5, 2016

Contributor

📌 Commit 5e9b555 has been approved by str4d

Contributor

zkbot commented Nov 5, 2016

📌 Commit 5e9b555 has been approved by str4d

@zkbot

This comment has been minimized.

Show comment
Hide comment
@zkbot

zkbot Nov 5, 2016

Contributor

⌛️ Testing commit 5e9b555 with merge 2648902...

Contributor

zkbot commented Nov 5, 2016

⌛️ Testing commit 5e9b555 with merge 2648902...

zkbot pushed a commit that referenced this pull request Nov 5, 2016

zkbot
Auto merge of #1778 - bitcartel:1762_segfault_miner, r=str4d
Fixes #1762 segfault when miner is interrupted.

Closes #1762

@str4d str4d modified the milestones: 1.0.3, 1.0.2 Nov 5, 2016

@zkbot

This comment has been minimized.

Show comment
Hide comment
@zkbot

zkbot Nov 6, 2016

Contributor

☀️ Test successful - zcash

Contributor

zkbot commented Nov 6, 2016

☀️ Test successful - zcash

@zkbot zkbot merged commit 5e9b555 into zcash:master Nov 6, 2016

1 check passed

homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment