Skip to content

[Core] Properly shut down veild if init fails.#935

Merged
codeofalltrades merged 1 commit intoVeil-Project:masterfrom
Zannick:shutdown
Apr 26, 2021
Merged

[Core] Properly shut down veild if init fails.#935
codeofalltrades merged 1 commit intoVeil-Project:masterfrom
Zannick:shutdown

Conversation

@Zannick
Copy link
Collaborator

@Zannick Zannick commented Apr 21, 2021

Problem

veild doesn't shut down properly if it fails during init.

Root Cause

Background threads were hanging after being interrupted, because they were looking to ShutdownRequested(), which was only being set by StartShutdown() on SIGTERM.

Solution

Solution is to call StartShutdown() when veild fails init and goes to interrupt running threads.

PR

#934

Testing

Set autospend=1 and autospendaddress=<basecoin address> (or make a similar error that should cause it to close with an error on startup) in your veil.conf and run veild with -printtoconsole.

Fixes Veil-Project#934: Background threads were hanging after being interrupted,
because they were looking to `ShutdownRequested()`, which was only
being set by `StartShutdown()` on SIGTERM. Solution is to call
`StartShutdown()` when veild fails init and goes to interrupt running
threads.
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.

ACK a66022a
Tested on Win 10

@codeofalltrades codeofalltrades added Component: Core App Related to the application itself. Tag: Waiting For Code Review Waiting for code review from a core developer labels Apr 24, 2021
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.

ACK a66022a

Must be merged before #923

@CaveSpectre11 CaveSpectre11 added Code Review: Passed and removed Tag: Waiting For Code Review Waiting for code review from a core developer labels Apr 25, 2021
@codeofalltrades codeofalltrades merged commit a440ed6 into Veil-Project:master Apr 26, 2021
@Zannick Zannick deleted the shutdown branch May 18, 2021 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Code Review: Passed Component: Core App Related to the application itself.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants