Skip to content

init: remove boost from ThreadImport.#780

Merged
codeofalltrades merged 1 commit intoVeil-Project:masterfrom
philmb3487:threadimport_no_boost
Jun 1, 2021
Merged

init: remove boost from ThreadImport.#780
codeofalltrades merged 1 commit intoVeil-Project:masterfrom
philmb3487:threadimport_no_boost

Conversation

@philmb3487
Copy link
Contributor

@philmb3487 philmb3487 commented May 7, 2020 •
edited
Loading

Problem
This is a refactor fix. We want to remove boost functions slowly.

Root Cause

Solution
We can work around the boost interruption point by just returning, and shutting down the program, instead. It is much cleaner than relying on boost to redirect control flow.

Unit Testing Results
Can be tested by calling -reindex or -loadblock and then pressing CTRL+C.

Should print something like:

...
2020-04-27T19:34:31Z [loadblk] Reindexing block file blk00005.dat...
^C2020-04-27T19:34:32Z [loadblk] Shutdown requested. Exit ThreadImport
2020-04-27T19:34:32Z [qt-init] Interrupting HTTP server
...

CaveSpectre11
CaveSpectre11 previously requested changes May 7, 2020
View reviewed changes
Copy link
Collaborator

@CaveSpectre11 CaveSpectre11 left a comment

Choose a reason for hiding this comment

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

Overall looks fine; but please maintain the bool type on LoadExternalBlockFile for future cleanup

@CaveSpectre11 CaveSpectre11 added the Tag: Waiting For Code Review Waiting for code review from a core developer label May 7, 2020
@philmb3487
Copy link
Contributor Author

philmb3487 commented May 7, 2020

Travis early failed due to caching I believe..

@philmb3487 philmb3487 added the Priority: 1 - Low Non-critical, low impact label May 7, 2020
mimirmim
mimirmim previously requested changes May 7, 2020
View reviewed changes
@codeofalltrades
Copy link
Collaborator

codeofalltrades commented May 8, 2020

Travis early failed due to caching I believe..
Yep, reran. All passing now.

codeofalltrades
codeofalltrades approved these changes May 8, 2020
View reviewed changes
Copy link
Collaborator

@codeofalltrades codeofalltrades left a comment

Choose a reason for hiding this comment

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

utACK 3660e40
assuming the other reviewer's comments to corrected.

@CaveSpectre11 CaveSpectre11 added Tag: Waiting For Developer and removed Tag: Waiting For Code Review Waiting for code review from a core developer labels May 8, 2020
@philmb3487
Copy link
Contributor Author

philmb3487 commented May 24, 2020

Addressed the issues, ready for another review.

@philmb3487 philmb3487 added Tag: Waiting For Code Review Waiting for code review from a core developer and removed Tag: Waiting For Developer labels May 24, 2020
@philmb3487 philmb3487 dismissed stale reviews from mimirmim and CaveSpectre11 May 24, 2020 16:48

done

CaveSpectre11
CaveSpectre11 approved these changes Jun 20, 2020
View reviewed changes
Copy link
Collaborator

@CaveSpectre11 CaveSpectre11 left a comment

Choose a reason for hiding this comment

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

utACK f2bbec8

@CaveSpectre11 CaveSpectre11 requested a review from mimirmim June 27, 2020 14:06
mimirmim
mimirmim approved these changes Jul 13, 2020
View reviewed changes
Copy link
Contributor

@mimirmim mimirmim left a comment

Choose a reason for hiding this comment

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

utACK f2bbec8

@CaveSpectre11 CaveSpectre11 added Code Review: Passed Tag: Waiting For QA A pull review is waiting for QA to test the pull request and removed Tag: Waiting For Code Review Waiting for code review from a core developer labels Aug 2, 2020
@seanPhill
Copy link
Collaborator

seanPhill commented Sep 15, 2020

ACK
With this PR #780 I've done numerous closes of the veil daemon (and of the Qt wallet) (with or without -reindex), with ctrl-c or otherwise and they all end without problem, with something like:

2020-09-15T00:00:38Z ThreadStaging exiting
2020-09-15T00:00:38Z tor: Thread interrupt
2020-09-15T00:00:38Z torcontrol thread exit
2020-09-15T00:00:38Z dnsseed thread exit
2020-09-15T00:00:38Z addcon thread exit
2020-09-15T00:00:38Z Shutdown: In progress...
2020-09-15T00:00:38Z UPNP_DeletePortMapping() returned: 0
2020-09-15T00:00:38Z upnp thread exit
2020-09-15T00:00:38Z msghand thread exit
2020-09-15T00:00:38Z net thread exit
2020-09-15T00:00:38Z opencon thread exit
2020-09-15T00:00:38Z Shutdown requested. Exit ThreadImport
2020-09-15T00:00:38Z ThreadStakeMiner() interrupted
2020-09-15T00:00:38Z ThreadStakeMiner exiting
2020-09-15T00:00:38Z scheduler thread interrupt
2020-09-15T00:00:38Z Shutdown: done

@CaveSpectre11 CaveSpectre11 added QA: Passed This has passed QA testing and can be merged to master and removed Tag: Waiting For QA A pull review is waiting for QA to test the pull request labels Sep 15, 2020
@codeofalltrades codeofalltrades merged commit 440184c into Veil-Project:master Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CaveSpectre11 CaveSpectre11 CaveSpectre11 approved these changes

@codeofalltrades codeofalltrades codeofalltrades approved these changes

+1 more reviewer

@mimirmim mimirmim mimirmim approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@philmb3487 philmb3487

Labels

Code Review: Passed Priority: 1 - Low Non-critical, low impact QA: Passed This has passed QA testing and can be merged to master

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@philmb3487 @codeofalltrades @seanPhill @CaveSpectre11 @mimirmim