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

Add Windows shutdown handler #13131

Merged
merged 1 commit into from May 7, 2018

Conversation

Projects
None yet
6 participants
@ken2812221
Copy link
Member

ken2812221 commented Apr 30, 2018

Exit properly when clicked the red X of Windows Console

@MarcoFalke MarcoFalke added the Windows label Apr 30, 2018

src/init.cpp Outdated
static BOOL WINAPI consoleCtrlHandler(DWORD dwCtrlType)
{
fRequestShutdown = true;
Sleep(-1);

This comment has been minimized.

@laanwj

laanwj May 1, 2018

Member

What does sleeping a negative amount of time do?

This comment has been minimized.

@ken2812221

ken2812221 May 1, 2018

Author Member

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

This comment has been minimized.

@laanwj

laanwj May 1, 2018

Member

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?

This comment has been minimized.

@ken2812221

ken2812221 May 1, 2018

Author Member

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

@ken2812221 ken2812221 force-pushed the ken2812221:win-quit branch to ddebde7 May 1, 2018

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented May 1, 2018

utACK ddebde7

@fanquake

This comment has been minimized.

Copy link
Member

fanquake commented May 1, 2018

Will test shortly.

@fanquake

This comment has been minimized.

Copy link
Member

fanquake commented May 2, 2018

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

@jonasschnelli

This comment has been minimized.

Copy link
Member

jonasschnelli commented May 2, 2018

Thanks. I think this was long overdue.
utACK ddebde7

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

@donaloconnor
Copy link
Contributor

donaloconnor left a comment

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

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request May 7, 2018

Merge #13131: Add Windows shutdown handler
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

This comment has been minimized.

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

This comment has been minimized.

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 ken2812221:win-quit branch May 7, 2018

@laanwj laanwj added this to the 0.16.2 milestone May 28, 2018

fanquake added a commit to fanquake/bitcoin that referenced this pull request Jun 13, 2018

Add Windows shutdown handler
GitHub-Pull: bitcoin#13131
Rebased-From: ddebde7

@fanquake fanquake referenced this pull request Jun 13, 2018

Merged

[0.16.2] Backports #13455

@fanquake

This comment has been minimized.

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

Merge #13455: [0.16.2] Backports
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

HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Jan 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.