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

os, runtime: CTRL_CLOSE_EVENT should generate SIGHUP on Windows #6948

Open
anacrolix opened this Issue Dec 13, 2013 · 5 comments

Comments

Projects
None yet
6 participants
@anacrolix
Contributor

anacrolix commented Dec 13, 2013

On Windows, the CTRL_C_EVENT and CTRL_BREAK_EVENT control signals are converted to
SIGINT in the runtime package for Windows for use by the os/signal package. After some
investigation, I believe CTRL_CLOSE_EVENT should be emulated as SIGHUP. SIGHUP is
already invented for compatibility purposes in the syscall package, in the same way that
SIGINT is, and then actively used by the existing handlers installed for CTRL_C_EVENT
and CTRL_BREAK_EVENT above.

Relevant lines in tip's source:

runtime·stdcall(runtime·SetConsoleCtrlHandler, 2, runtime·ctrlhandler, (uintptr)1);
// This registers Go's control event handler to convert control events into unix-like
signals

/* in the handler these events are converted to SIGINT */
    case CTRL_C_EVENT:
    case CTRL_BREAK_EVENT:
        s = SIGINT;

/* invented values for important signals */
    SIGHUP  = Signal(0x1)
    SIGINT  = Signal(0x2)

Please also see:

http://stackoverflow.com/q/20511182/149482
http://msdn.microsoft.com/en-us/library/windows/desktop/ms685049%28v=vs.85%29.aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/ms683242(v=vs.85).aspx
@anacrolix

This comment has been minimized.

Contributor

anacrolix commented Dec 15, 2013

Comment 1:

I implemented the required changes in the Go code base and verified it works. If there
is interest in this fix I can submit a proper patch via Rietveld.
@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Dec 15, 2013

Comment 2:

Sound good to me. Please do. Thank you.
Alex
@davecheney

This comment has been minimized.

Contributor

davecheney commented Feb 3, 2014

Comment 3:

@anacrolix please propose a change.

Labels changed: added release-none, repo-main, suggested.

Status changed to Started.

@anacrolix

This comment has been minimized.

Contributor

anacrolix commented Feb 17, 2014

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Feb 18, 2014

Comment 5:

just to store image

Attachments:

  1. alex.JPG (17991 bytes)

@rsc rsc added this to the Unplanned milestone Apr 10, 2015

@rsc rsc removed release-none labels Apr 10, 2015

@bradfitz bradfitz removed the Started label Jan 6, 2017

@mvdan mvdan added the OS-Windows label Jan 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment