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 SIGTERM on Windows #7479

Open
gopherbot opened this Issue Mar 6, 2014 · 24 comments

Comments

Projects
None yet
10 participants
@gopherbot

gopherbot commented Mar 6, 2014

by anacrolix@google.com:

On Windows a CTRL_CLOSE_EVENT message is sent prior to unavoidable termination. This
should be represented by notifying the process with a SIGTERM.

http://msdn.microsoft.com/en-us/library/windows/desktop/ms683242(v=vs.85).aspx
@gopherbot

This comment has been minimized.

gopherbot commented Apr 8, 2014

Comment 1 by sgellerde:

fyi,
after applying [1] and doing dev tests it looks like CTRL_CLOSE_EVENT can only be
received for a console process in Windows 7 when using some nasty hacks [2]. Pre Win7
the event seems receivable. If desired I would test it under Win8+. Or did I miss
anything obvious in my diff?
[1] https://gist.github.com/sgeller/10196571
[2]
http://social.msdn.microsoft.com/Forums/windowsdesktop/en-US/abf09824-4e4c-4f2c-ae1e-5981f06c9c6e/windows-7-console-application-has-no-way-of-trapping-logoffshutdown-event?forum=windowscompatibility
@gopherbot

This comment has been minimized.

gopherbot commented Apr 9, 2014

Comment 2 by anacrolix@google.com:

That's strange it works for me and I'm using Windows 7 32bit. From the thread it looks
like the big spanners you can throw in are:
 * Your app is a GUI/full-fledge Win32 app (this requires a flag to the go linker).
 * You're invoking other Win32 APIs such as service-related and event-loop related stuff.
None of the above were true in my testing.
The patch you want is https://codereview.appspot.com/download/issue71900043_40001.diff
Your gist generates SIGINT for CTRL_CLOSE_EVENT which isn't optimal, the patch generates
SIGTERM for reasons given in the source. Either way you must also catch the signal with
os.Notify. There's a test program here:
https://bitbucket.org/anacrolix/dms/src/8d6ab94e3c8032d11b870867609ba9dd618e79fc/play/termsig/?at=default
If you pipe the output to a file from a terminal, and then close the terminal you should
see the termination signal in the file.
@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented May 7, 2014

Comment 3:

Labels changed: added repo-main, release-none, os-windows.

@gopherbot

This comment has been minimized.

gopherbot commented Sep 26, 2014

Comment 5 by RetroModular:

FYI // I have just been having a discussion about this with some guys in one of the Go
Google groups because I was looking for a solution to catch the console being closed (on
Windows) while a Go app is running.
This is the discussion:
https://groups.google.com/forum/#!topic/golang-nuts/S_RDVG3lcdI
Java appears to be handling this in a sensible way by listening for CLOSE and SHUTDOWN
events then firing a SIGTERM signal:
http://hg.openjdk.java.net/jdk9/dev/hotspot/file/9b3f5e4f3372/src/os/windows/vm/os_windows.cpp#l1883
It would be really helpful if this could be implemented in Go.
@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Sep 29, 2014

Comment 6:

I am happy to try and implement this functionality (after go1.4 is released), but I need
some input:
- which extra events we should handle?
- which syscall or os signals these new events should me mapped into?
I have very little experience with these, and I don't want to influence that decision.
Alex
@flowchartsman

This comment has been minimized.

flowchartsman commented Jul 16, 2015

So is this issue effectively dead? It would be great to be able to handle certain windows-specific signals.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Jul 16, 2015

The issue is not dead, but I'm not aware of anybody working on it.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Jul 16, 2015

@Alaska what are you trying to do?

Alex

@flowchartsman

This comment has been minimized.

flowchartsman commented Jul 17, 2015

To detect (as best as I can) when the window is closed on a console application. No matter how it's represented internally, CTRL_CLOSE_EVENT is useful to capture so that cleanup actions can be performed, like flushing to disk or gracefully disconnecting from a service in the 5 seconds allotted after the signal is sent.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Jul 17, 2015

Thank you. I will try and implement this once Go source tree opens again. Unless you want to do it yourself.

Alex

@flowchartsman

This comment has been minimized.

flowchartsman commented Jul 17, 2015

I'd be happy to, though I would like a collaborator or owner to have the final say on what it maps to, though SIGTERM does seem reasonable.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Jul 17, 2015

I really don't know. I suggest we go with SIGTERM. Others will correct us, if they want to.

Alex

@mattn

This comment has been minimized.

Member

mattn commented Jul 17, 2015

+1 to SIGTERM

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Sep 9, 2015

I tried implementing this (windows-386 only):

diff --git a/src/runtime/defs_windows_386.go b/src/runtime/defs_windows_386.go
index bac6ce7..04c6d7c 100644
--- a/src/runtime/defs_windows_386.go
+++ b/src/runtime/defs_windows_386.go
@@ -16,8 +16,10 @@ const (
    _THREAD_PRIORITY_HIGHEST = 0x2

    _SIGINT           = 0x2
+   _SIGTERM          = 0xf
    _CTRL_C_EVENT     = 0x0
    _CTRL_BREAK_EVENT = 0x1
+   _CTRL_CLOSE_EVENT = 0x2

    _CONTEXT_CONTROL = 0x10001
    _CONTEXT_FULL    = 0x10007
diff --git a/src/runtime/os1_windows.go b/src/runtime/os1_windows.go
index f608b4a..8315375 100644
--- a/src/runtime/os1_windows.go
+++ b/src/runtime/os1_windows.go
@@ -448,19 +448,20 @@ func usleep(us uint32) {
 }

 func ctrlhandler1(_type uint32) uint32 {
-   var s uint32
-
    switch _type {
    case _CTRL_C_EVENT, _CTRL_BREAK_EVENT:
-       s = _SIGINT
-   default:
-       return 0
-   }
-
-   if sigsend(s) {
-       return 1
+       if sigsend(_SIGINT) {
+           return 1
+       }
+       exit(2)
+   case _CTRL_CLOSE_EVENT:
+       if sigsend(_SIGTERM) {
+           // should not return from ctrlhandler1,
+           // otherwise windows will terminate us immediately.
+           usleep(100 * 1000000)
+       }
+       exit(2)
    }
-   exit(2) // SIGINT, SIGTERM, etc
    return 0
 }

and the only way I can stop Windows closing my process is by not returning from ctrlhandler1 (see usleep I have inserted there). This way everything works as expected - I get new signal and can act on it as I wish.

Unfortunately my change has one bad side effect. I cannot close console window with my process running in it now (unless I have Go SIGTERM handler doing exiting explicitly). We cannot have this change of default behaviour.

I don't see how we can overcome this.

Alex

@mattn

This comment has been minimized.

Member

mattn commented Sep 10, 2015

I'm thinking, this issue is how to handle SIGTERM on windows using CTRL_CLOSE_EVENT.So it should be a default behavior if the if the user does not catch SIGTERM.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Sep 11, 2015

But none of my programs handle SIGTERM. Sorry @mattn, but I don't like that suddenly all my Go programs won't allow me to close console window. We have to find a way around this problem.

Alex

@nabeelomer

This comment has been minimized.

nabeelomer commented Nov 10, 2015

To handle control events, one can register a Control Handler Function which can be easily done syscalls.

SetConsoleCtrlHandler((PHANDLER_ROUTINE) CtrlHandler, TRUE);

Is there a reason why nobody has suggested that?

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Nov 11, 2015

To handle control events, one can register a Control Handler Function which can be easily done syscalls.
SetConsoleCtrlHandler((PHANDLER_ROUTINE) CtrlHandler, TRUE);
Is there a reason why nobody has suggested that?

Go runtime uses SetConsoleCtrlHandler to handle CTRL_C_EVENT and CTRL_BREAK_EVENT events. (grep runtime package for SetConsoleCtrlHandler). I don't understand why someone has to suggest to use SetConsoleCtrlHandler.

Alex

@rgl

This comment has been minimized.

rgl commented Apr 2, 2018

Can we please have a way to catch CTRL_SHUTDOWN_EVENT (and not return from the handler)? This is the only signal that is currently (Windows 1803+) sent to a console process that is running inside a Windows Container.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Apr 6, 2018

Can we please have a way to catch CTRL_SHUTDOWN_EVENT (and not return from the handler)?

Did you see #7479 (comment) ? I suspect you will have the same problem with CTRL_SHUTDOWN_EVENT. Feel free to try it yourself.

Alex

@maruel

This comment has been minimized.

Contributor

maruel commented Apr 6, 2018

@alexbrainman if I understand correctly, the problem lies into making sure that sigsend() is completely synchronous? Maybe call signalWaitUntilIdle()?

Potentially naive pseudo code:

func ctrlhandler1(_type uint32) uint32 {
  switch _type {
  case _CTRL_C_EVENT, _CTRL_BREAK_EVENT:
    if !sigsend(_SIGINT) {
      exit(2) // Signal processing failed.
    }
    return 1 // Disallow default processing to not terminate the process.
  case CTRL_CLOSE_EVENT:
    if !sigsend(_SIGTERM) {
      exit(2) // Signal processing failed.
    }
    signalWaitUntilIdle()
    return 1 // Windows will terminate the process forcibly.
  default:
    return 0 // Default handler will terminate the process.
  }
}
@rgl

This comment has been minimized.

rgl commented Apr 6, 2018

@alexbrainman I did see it, but I was looking for an out-of-the-box solution. i.e. I do not want to compile my own version of go.

Maybe we can get around this without re-compiling go? Maybe by calling SetConsoleCtrlHandler from our own program? In theory, Windows, supports multiple handlers. Have you tried it?

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Apr 7, 2018

if I understand correctly, the problem lies into making sure that sigsend() is completely synchronous?

@maruel the problem with my change from #7479 (comment) is that it requires every single Go program to handle SIGTERM. For example, if I run this program

package main

import "time"

func main() {
	time.Sleep(time.Hour)
}

in a Windows command window, and then click close button of that window, nothing happens.

Maybe call signalWaitUntilIdle()?

Maybe it will, but I don't see how.

Maybe we can get around this without re-compiling go? Maybe by calling SetConsoleCtrlHandler from our own program? In theory, Windows, supports multiple handlers. Have you tried it?

@rgl I did not try calling SetConsoleCtrlHandler from Go.

Alex

@rgl

This comment has been minimized.

rgl commented Apr 8, 2018

@alexbrainman I tried with the following snippet, but for some reason, the callback is never called (altough the CTRL+C shortcut stops working). Do you have any idea why its not working?

package main

import (
	"log"
	"time"

	"syscall"

	"golang.org/x/sys/windows"
)

func main() {
	kernel32 := windows.NewLazySystemDLL("kernel32.dll")

	setConsoleCtrlHandler := kernel32.NewProc("SetConsoleCtrlHandler")

	r1, r2, lastErr := setConsoleCtrlHandler.Call(
		syscall.NewCallback(func(controlType uint) uint {
			log.Printf("consoleControlHandler called with %v", controlType)
			panic("ops")
			return 0
		}),
		1)
	log.Printf("call result %v %v %v", r1, r2, lastErr)

	for {
		time.Sleep(1 * time.Second)
	}
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment