Skip to content

Add Windows shutdown handler #13131

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
Merged
laanwj merged 1 commit into from
May 7, 2018
Merged

Add Windows shutdown handler #13131

laanwj merged 1 commit into from
May 7, 2018

Conversation

@ken2812221
Copy link
Contributor

@ken2812221 ken2812221 commented Apr 30, 2018

Exit properly when clicked the red X of Windows Console

laanwj
laanwj reviewed May 1, 2018
View reviewed changes
src/init.cpp Outdated
Copy link
Member

@laanwj laanwj May 1, 2018

Choose a reason for hiding this comment

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

What does sleeping a negative amount of time do?

Copy link
Contributor Author

@ken2812221 ken2812221 May 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Sleep as long as possible and wait for main thread quit.

Copy link
Member

@laanwj laanwj May 1, 2018

Choose a reason for hiding this comment

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

I've checked MSDN docs: https://msdn.microsoft.com/en-us/library/windows/desktop/ms686298(v=vs.85).aspx and the parameter is a DWORD (unsigned) to -1 is not a valid value.
It seems that there is a constant INFINITE to wait infinitely, if that is what is the intent?

Copy link
Contributor Author

@ken2812221 ken2812221 May 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

INFINITE is 0xFFFFFFFF, so it might be the same. But to be safe, I would change it to the macro.

@laanwj
Copy link
Member

laanwj commented May 1, 2018

utACK ddebde7

@fanquake
Copy link
Member

fanquake commented May 1, 2018

Will test shortly.

@fanquake
Copy link
Member

fanquake commented May 2, 2018

@jonasschnelli Could you spin up a Windows build for this PR?

@jonasschnelli
Copy link
Contributor

jonasschnelli commented May 2, 2018

Thanks. I think this was long overdue.
utACK ddebde7

Gitian Build: https://bitcoin.jonasschnelli.ch/build/591

donaloconnor
donaloconnor approved these changes May 5, 2018
View reviewed changes
Copy link
Contributor

@donaloconnor donaloconnor left a comment •
edited
Loading

Choose a reason for hiding this comment

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

tACK ddebde7

I had this change also but didn't get around to submitting it :-)

This works generally but CTRL_CLOSE_EVENT times out after 5 seconds on my OS (Win10) if the main thread hasn't ended gracefully. At that point all threads are terminated.

@laanwj laanwj merged commit ddebde7 into bitcoin:master May 7, 2018
laanwj added a commit that referenced this pull request May 7, 2018
@laanwj
Merge #13131: Add Windows shutdown handler
57aae63 
ddebde7 Add Windows shutdown handler (Chun Kuan Lee)

Pull request description:

  Exit properly when clicked the red X of Windows Console

Tree-SHA512: f030edd08868390662b42abfa1dc6bd702166c6c19f5b1f8e7482e202451e79fb6f37ea672c26c2eb0d32c367bfca86160fbee624696c53828f280b7070be6a0
@laanwj
Copy link
Member

laanwj commented May 7, 2018

This works generally but CTRL_CLOSE_EVENT times out after 5 seconds on my OS (Win10) if the main thread hasn't ended gracefully. At that point all threads are terminated.

I guess there's nothing to be done against that, and it's the same as on UNIX. Every OS has a timeout after which it loses patience and really terminates a process during shutdown.

@donaloconnor
Copy link
Contributor

donaloconnor commented May 7, 2018

I guess there's nothing to be done against that, and it's the same as on UNIX. Every OS has a timeout after which it loses patience and really terminates a process during shutdown

Indeed! Just putting there for FYI really.

@ken2812221 ken2812221 deleted the win-quit branch May 7, 2018 18:09
@Bushstar Bushstar mentioned this pull request May 8, 2018
Merged
@laanwj laanwj added this to the 0.16.2 milestone May 28, 2018
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Jun 13, 2018
@ken2812221 @fanquake
Add Windows shutdown handler
ce8aa54 
GitHub-Pull: bitcoin#13131
Rebased-From: ddebde7
@fanquake fanquake mentioned this pull request Jun 13, 2018
Merged
@fanquake
Copy link
Member

fanquake commented Jun 26, 2018

Added to #13455 for backport.

laanwj added a commit that referenced this pull request Jul 9, 2018
@laanwj
Merge #13455: [0.16.2] Backports
dac5374 
9fd3e00 depends: Update Qt download url (fanquake)
f7401c8 Fix parameter count check for importpubkey. (Kristaps Kaupe)
cbd2f70 expose CBlockIndex::nTx in getblock(header) (Gregory Sanders)
ce8aa54 Add Windows shutdown handler (Chun Kuan Lee)
18b0c69 Bugfix: Include <memory> for std::unique_ptr (Luke Dashjr)

Pull request description:

  Backports:
  * #12859 Bugfix: Include <memory> for std::unique_ptr
  * #13131 Add Windows shutdown handler
  * #13451 rpc: expose CBlockIndex::nTx in getblock(header)
  * #13507 RPC: Fix parameter count check for importpubkey
  * #13544 depends: Update Qt download url

  to the 0.16 branch.

Tree-SHA512: eeaec52d001d5c81e67dda3a2d3fee7a9445e569366e597b18e81d802c1b7f89e545afd53d094740c37c1714050304979398b9860144454d3a5cb5abc9e9eaca
@jamescowens jamescowens mentioned this pull request Sep 25, 2018
Merged
HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Jan 11, 2019
@ken2812221
Add Windows shutdown handler
b2e5de4 
GitHub-Pull: bitcoin#13131
Rebased-From: ddebde7
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 8, 2020
@laanwj @PastaPastaPasta
Merge bitcoin#13131: Add Windows shutdown handler
11b6221 
ddebde7 Add Windows shutdown handler (Chun Kuan Lee)

Pull request description:

  Exit properly when clicked the red X of Windows Console

Tree-SHA512: f030edd08868390662b42abfa1dc6bd702166c6c19f5b1f8e7482e202451e79fb6f37ea672c26c2eb0d32c367bfca86160fbee624696c53828f280b7070be6a0
@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

@laanwj laanwj laanwj left review comments

+1 more reviewer

@donaloconnor donaloconnor donaloconnor approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Windows

Projects

None yet

Milestone

0.16.2

Development

Successfully merging this pull request may close these issues.

6 participants

@ken2812221 @laanwj @fanquake @jonasschnelli @donaloconnor @maflcko