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.

Sign up for GitHub

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

Jump to bottom

Always return true if AppInitMain got to the end #11831

Merged
merged 1 commit into from Dec 12, 2017
Merged

Always return true if AppInitMain got to the end #11831

merged 1 commit into from Dec 12, 2017

Conversation

TheBlueMatt
Copy link
Contributor

This should fix a rare zapwallettxes failure on travis, but also
avoids having init operations (re-adding wallet transactions to
mempool) running after RPC is free'd.

I believe this was the failure at https://travis-ci.org/bitcoin/bitcoin/jobs/311747844 (from #11605).

@TheBlueMatt
Always return true if AppInitMain got to the end
07c4838 
This should fix a very rare travis failure in zapwallettxes, but
is also more correct, as you can currently race
ReacceptWalletTransactions with stop RPC calls to get bitcoind to
(IMO) eroneously return a non-0 exit code.
promag
promag reviewed Dec 5, 2017
View reviewed changes
Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

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

ACK 07c4838.

Shutdown is handled after AppInitMain(). If the initialisation succeeded or not shouldn't depend on fRequestShutdown.

@TheBlueMatt TheBlueMatt mentioned this pull request Dec 5, 2017
Merged
@meshcollider
Copy link
Contributor

utACK 07c4838

@sipa
Copy link
Member

sipa commented Dec 6, 2017

utACK 07c4838

@laanwj
Copy link
Member

laanwj commented Dec 6, 2017 •
edited

If the initialisation succeeded or not shouldn't depend on fRequestShutdown.

"Terminated during initialization" is a valid reason for initialization to fail. The reason for returning false here is that initialization didn't fully finish, which is used in the GUI code to (not) spin up other things.

@promag
Copy link
Member

promag commented Dec 6, 2017

Yes but only if something was interrupted in the initialisation, that's not the case here, where everything was initialised.

@laanwj
Copy link
Member

laanwj commented Dec 6, 2017

Yes but only if something was interrupted in the initialisation, that's not the case here, where everything was initialised.

So in the case something was actually interrupted, it still early-outs with false? Then this change is fine with me.

@TheBlueMatt
Copy link
Contributor Author

Indeed, if initialization did not finish because of fRequestShutdown, false is returned after block index loading fails to complete.

@ryanofsky ryanofsky mentioned this pull request Dec 6, 2017
Merged
@laanwj laanwj merged commit 07c4838 into bitcoin:master Dec 12, 2017
laanwj added a commit that referenced this pull request Dec 12, 2017
@laanwj
Merge #11831: Always return true if AppInitMain got to the end
d48ab83 
07c4838 Always return true if AppInitMain got to the end (Matt Corallo)

Pull request description:

  This should fix a rare zapwallettxes failure on travis, but also
  avoids having init operations (re-adding wallet transactions to
  mempool) running after RPC is free'd.

  I believe this was the failure at https://travis-ci.org/bitcoin/bitcoin/jobs/311747844 (from #11605).

Tree-SHA512: f0fea8c1b9265e2eeda57043d541380a3e58e4d9388fa24628a52fd56324257fcd7df0ca02e8f77f66fadd68d951893bab0f610ed9fd0a89b2ccd6bad1efa351
@maflcko maflcko mentioned this pull request Dec 17, 2017
Closed
@promag promag mentioned this pull request Feb 7, 2018
Merged
laanwj added a commit that referenced this pull request Feb 8, 2018
@laanwj
Merge #12377: qt: Poll ShutdownTimer after init is done
36a927c 
2222bf0 qt: Poll ShutdownTimer after init is done (MarcoFalke)

Pull request description:

  The shutdown process has started in `requestShutdown`, but initialize will happily continue with `initializeResult` and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
  https://github.com/qt/qtbase/blob/e5033a5c9b769815112e922d0b224af860afd219/src/corelib/thread/qthread.cpp#L412-L415

  Potential fix for #12372 (comment)

  This reverts #11831 for now and hopefully restores the previous behaviour.

Tree-SHA512: 8e1706afe90ddf2d972aca12c12d4cb2a9a4f38646c59c5466fe5a1a67361896b93c43917d5ac283841ee2bcc62e6bb8dc2bc81dea9129c899b354e9a4ef241b
@dagurval dagurval mentioned this pull request Jun 14, 2018
Merged
@sickpig sickpig mentioned this pull request Jun 15, 2018
Merged
@codablock codablock mentioned this pull request Jun 25, 2019
Merged
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 25, 2019
@laanwj @PastaPastaPasta
Merge bitcoin#11831: Always return true if AppInitMain got to the end
60475f1 
07c4838 Always return true if AppInitMain got to the end (Matt Corallo)

Pull request description:

  This should fix a rare zapwallettxes failure on travis, but also
  avoids having init operations (re-adding wallet transactions to
  mempool) running after RPC is free'd.

  I believe this was the failure at https://travis-ci.org/bitcoin/bitcoin/jobs/311747844 (from bitcoin#11605).

Tree-SHA512: f0fea8c1b9265e2eeda57043d541380a3e58e4d9388fa24628a52fd56324257fcd7df0ca02e8f77f66fadd68d951893bab0f610ed9fd0a89b2ccd6bad1efa351
barrystyle pushed a commit to PACGlobalOfficial/PAC that referenced this pull request Jan 22, 2020
@laanwj @barrystyle
Merge bitcoin#11831: Always return true if AppInitMain got to the end
c084ef2 
07c4838 Always return true if AppInitMain got to the end (Matt Corallo)

Pull request description:

  This should fix a rare zapwallettxes failure on travis, but also
  avoids having init operations (re-adding wallet transactions to
  mempool) running after RPC is free'd.

  I believe this was the failure at https://travis-ci.org/bitcoin/bitcoin/jobs/311747844 (from bitcoin#11605).

Tree-SHA512: f0fea8c1b9265e2eeda57043d541380a3e58e4d9388fa24628a52fd56324257fcd7df0ca02e8f77f66fadd68d951893bab0f610ed9fd0a89b2ccd6bad1efa351
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11831: Always return true if AppInitMain got to the end
64aba48 
07c4838 Always return true if AppInitMain got to the end (Matt Corallo)

Pull request description:

  This should fix a rare zapwallettxes failure on travis, but also
  avoids having init operations (re-adding wallet transactions to
  mempool) running after RPC is free'd.

  I believe this was the failure at https://travis-ci.org/bitcoin/bitcoin/jobs/311747844 (from bitcoin#11605).

Tree-SHA512: f0fea8c1b9265e2eeda57043d541380a3e58e4d9388fa24628a52fd56324257fcd7df0ca02e8f77f66fadd68d951893bab0f610ed9fd0a89b2ccd6bad1efa351
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 27, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11831: Always return true if AppInitMain got to the end
8e1572d 
07c4838 Always return true if AppInitMain got to the end (Matt Corallo)

Pull request description:

  This should fix a rare zapwallettxes failure on travis, but also
  avoids having init operations (re-adding wallet transactions to
  mempool) running after RPC is free'd.

  I believe this was the failure at https://travis-ci.org/bitcoin/bitcoin/jobs/311747844 (from bitcoin#11605).

Tree-SHA512: f0fea8c1b9265e2eeda57043d541380a3e58e4d9388fa24628a52fd56324257fcd7df0ca02e8f77f66fadd68d951893bab0f610ed9fd0a89b2ccd6bad1efa351
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 27, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#11831: Always return true if AppInitMain got to the end
3855979 
07c4838 Always return true if AppInitMain got to the end (Matt Corallo)

Pull request description:

  This should fix a rare zapwallettxes failure on travis, but also
  avoids having init operations (re-adding wallet transactions to
  mempool) running after RPC is free'd.

  I believe this was the failure at https://travis-ci.org/bitcoin/bitcoin/jobs/311747844 (from bitcoin#11605).

Tree-SHA512: f0fea8c1b9265e2eeda57043d541380a3e58e4d9388fa24628a52fd56324257fcd7df0ca02e8f77f66fadd68d951893bab0f610ed9fd0a89b2ccd6bad1efa351
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 14, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12377: qt: Poll ShutdownTimer after init is done
6b32a15 
2222bf0 qt: Poll ShutdownTimer after init is done (MarcoFalke)

Pull request description:

  The shutdown process has started in `requestShutdown`, but initialize will happily continue with `initializeResult` and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
  https://github.com/qt/qtbase/blob/e5033a5c9b769815112e922d0b224af860afd219/src/corelib/thread/qthread.cpp#L412-L415

  Potential fix for bitcoin#12372 (comment)

  This reverts bitcoin#11831 for now and hopefully restores the previous behaviour.

Tree-SHA512: 8e1706afe90ddf2d972aca12c12d4cb2a9a4f38646c59c5466fe5a1a67361896b93c43917d5ac283841ee2bcc62e6bb8dc2bc81dea9129c899b354e9a4ef241b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 14, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12377: qt: Poll ShutdownTimer after init is done
bf1d2ce 
2222bf0 qt: Poll ShutdownTimer after init is done (MarcoFalke)

Pull request description:

  The shutdown process has started in `requestShutdown`, but initialize will happily continue with `initializeResult` and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
  https://github.com/qt/qtbase/blob/e5033a5c9b769815112e922d0b224af860afd219/src/corelib/thread/qthread.cpp#L412-L415

  Potential fix for bitcoin#12372 (comment)

  This reverts bitcoin#11831 for now and hopefully restores the previous behaviour.

Tree-SHA512: 8e1706afe90ddf2d972aca12c12d4cb2a9a4f38646c59c5466fe5a1a67361896b93c43917d5ac283841ee2bcc62e6bb8dc2bc81dea9129c899b354e9a4ef241b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 15, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#12377: qt: Poll ShutdownTimer after init is done
7e5c5d4 
2222bf0 qt: Poll ShutdownTimer after init is done (MarcoFalke)

Pull request description:

  The shutdown process has started in `requestShutdown`, but initialize will happily continue with `initializeResult` and start threads late in the shutdown progess. Deleting this running thread will crash the application according to the qt docs:
  https://github.com/qt/qtbase/blob/e5033a5c9b769815112e922d0b224af860afd219/src/corelib/thread/qthread.cpp#L412-L415

  Potential fix for bitcoin#12372 (comment)

  This reverts bitcoin#11831 for now and hopefully restores the previous behaviour.

Tree-SHA512: 8e1706afe90ddf2d972aca12c12d4cb2a9a4f38646c59c5466fe5a1a67361896b93c43917d5ac283841ee2bcc62e6bb8dc2bc81dea9129c899b354e9a4ef241b
@furszy furszy mentioned this pull request Feb 16, 2021
Merged
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Feb 21, 2021
@random-zebra
Merge #2207: Use non-atomic flushing with block replay
ac52366 
aab15d7 ReplayBlocks: use find instead of brackets operator to access to the element. (furszy)
e898353 [Refactoring] Use const CBlockIndex* where appropriate (random-zebra)
c76fa04 qa: Extract rpc_timewait as test param (furszy)
0f832e3 shutdown: Stop threads before resetting ptrs (MarcoFalke)
67aebbf http: Remove numThreads and ThreadCounter (Wladimir J. van der Laan)
e24c710 http: Remove WaitExit from WorkQueue (Wladimir J. van der Laan)
b8f7364 http: Join worker threads before deleting work queue (Wladimir J. van der Laan)
7d68769 rpc: further constrain the libevent workaround (Cory Fields)
75af065 rpc: work-around an upstream libevent bug (Cory Fields)
50e5833 Always return true if AppInitMain got to the end (Matt Corallo)
bd70dcc [qa] Test non-atomic chainstate writes (furszy)
8f04970 Dont create pcoinsTip until after ReplayBlocks. (Matt Corallo)
93f2b15 Random db flush crash simulator (Pieter Wuille)
72f3b17 Adapt memory usage estimation for flushing (Pieter Wuille)
8540113 Non-atomic flushing using the blockchain as replay journal (Pieter Wuille)
8d6625f [MOVEONLY] Move LastCommonAncestor to chain (Pieter Wuille)

Pull request description:

  > This patch adds an extra "head blocks" to the chainstate, which gives the range of blocks for writes may be incomplete. At the start of a flush, we write this record, write the dirty dbcache entries in 16 MiB batches, and at the end we remove the heads record again. If it is present at startup it means we crashed during flush, and we rollback/roll forward blocks inside of it to get a consistent tip on disk before proceeding.

  > If a flush completes succesfully, the resulting database is compatible with previous versions. If the node crashes in the middle of a flush, a version of the code with this patch is needed to recovery.

  An adaptation of the following PRs with further modifications to the `feature_dbcrash.py` test to be up-to-date with upstream and solve RPC related bugs.

  * bitcoin#10148.
  * Increase RPC wait time.
  * bitcoin#11831
  * bitcoin#11593
  * bitcoin#12366
  * bitcoin#13837
  * bitcoin#13894

ACKs for top commit:
  random-zebra:
    ACK aab15d7
  Fuzzbawls:
    ACK aab15d7

Tree-SHA512: 898806746f581a9eb8deb0155c558481abf4454c6f3b3c3ad505c557938d0700fe9796e98e36492286ae869378647072c3ad77ad65e9dd7075108ff96469ade1
@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.
Reviewers

@promag promag promag left review comments

Assignees
No one assigned
Labels
Utils/log/libs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@TheBlueMatt @meshcollider @sipa @laanwj @promag