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

init: remove boost from ThreadImport. #780

Merged
merged 1 commit into from
Jun 1, 2021
Merged

init: remove boost from ThreadImport. #780

merged 1 commit into from
Jun 1, 2021

Conversation

sinetek
Copy link
Contributor

@sinetek sinetek commented May 7, 2020

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
...

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

src/validation.cpp Outdated Show resolved Hide resolved
src/init.cpp Show resolved Hide resolved
@CaveSpectre11 CaveSpectre11 added the Tag: Waiting For Code Review Waiting for code review from a core developer label May 7, 2020
@sinetek
Copy link
Contributor Author

sinetek commented May 7, 2020

Travis early failed due to caching I believe..

@sinetek sinetek added the Priority: 1 - Low Non-critical, low impact label May 7, 2020
src/validation.cpp Outdated Show resolved Hide resolved
@codeofalltrades
Copy link
Collaborator

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

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
@sinetek
Copy link
Contributor Author

sinetek commented May 24, 2020

Addressed the issues, ready for another review.

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

done

@CaveSpectre11 CaveSpectre11 added this to Ready for Code Review in Open PR Sprints Jun 13, 2020
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 moved this from Ready for Code Review to Code Review In Progress in Open PR Sprints Jun 27, 2020
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
@CaveSpectre11 CaveSpectre11 moved this from Code Review In Progress to Ready For QA in Open PR Sprints Aug 2, 2020
@seanPhill
Copy link
Collaborator

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
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
Open PR Sprints
Ready For QA
Development

Successfully merging this pull request may close these issues.

None yet

5 participants