uber-go/fx

unable to catch SIGINT in windows after v1.13.1

wesom opened this issue · 14 comments

wesom commented

During development, program didn't catch SIGINT by pressing Ctrl-C in windows. This was really frustrating. It runs normally under unix.

abhinav commented

Hello @wesom! Sorry about the delay. Can you confirm this is in v1.13.1, and not, 1.14.0? I can see the changes in 1.14.0 having a possibility of introducing this issue, but 1.13.1 would be surprising.

And either way, would you please include how you're initializing the application? And what version of Go you're using?

wesom commented

It runs normally in v1.13.1, but is interrupted with no output in v1.14.0. The cause of sigint loss was the use of windows.SIGINT instead of syscall.SIGINT. This has nothing to do with how to initialize the app or the version of Go.

abhinav commented

Hey @wesom, the version number is useful. This is definitely a bug in Fx.

Apologies, I didn't mean to suggest you're doing anything incorrectly in your application. I'm just trying to gather data to debug the issue.

  1. Depending on how you initialize the application (app.Run() vs app.Start() with manual use of app.Done()), we would be able to narrow down if the issue was in our signal registration logic or handling logic.
  2. We recently started including Go 1.17 compliant build annotations (//go:build) along with build tags (// +build). The Go version would help us narrow down whether this was an issue with the build tags not being respected.

So we'd appreciate it if you could share more information to help debug and fix the issue.

Thanks!

wesom commented

Hi @abhinav , sorry about being late.

  1. I jsut initialized the application using app.Run() but not app.Start() and app.Done().
    The app runs like this,
func runApp() {  
	app := fx.New(  
		fx.Provide(  
			library.NewLogger,  
			NewConfig,  
			server.NewServer,  
			jobs.NewCron,  
		),  
		fx.Invoke(server.Register, jobs.Register),  
		fx.WithLogger(  
			func(logger *zap.Logger) fxevent.Logger {  
				return &fxevent.ZapLogger{Logger: logger}  
			},  
		),  
	)  
	app.Run()  
}
  1. The Go version according to the app is 1.17 .
abhinav commented

Thanks, @wesom! We'll look into this.

Internal issue ref: GO-889

mpe85 commented

@abhinav
I'm experiencing the same problem with v1.14.2. Are there any news on this?

sywhang commented

@mpe85 sorry you're also running into this issue. I will take a look at this and address it soon.

Arekusei commented

@sywhang
Maybe it is issue in golang.org/x/sys/windows package.
It uses it's own type for Signals but Notify checks type of signal to convert it in int and it produce issue.
Code from signal_unix.go in standart library used to convert signal into int:

func signum(sig os.Signal) int {
	switch sig := sig.(type) {
	case syscall.Signal:
		i := int(sig)
		if i < 0 || i >= numSig {
			return -1
		}
		return i
	default:
		return -1
	}
}

What is the reason to use signal constants outside the standard library (package syscall)? It can solve this problem.

sywhang commented

@Arekusei I believe package syscall is deprecated and that is why we are using golang.org/x/sys/windows and golang.org/x/sys/unix instead:

Deprecated: this package is locked down. Callers should use the corresponding package in the golang.org/x/sys repository instead

@mpe85 @wesom do you have a minimal repro for this issue? I just tried to repro this on my Windows box and it seems to be working fine.

Here's the sample hello world program I used:

package main

import (
	"fmt"

	"go.uber.org/fx"
	"go.uber.org/fx/fxevent"
	"go.uber.org/zap"
)

func main() {
	app := fx.New(
		fx.Provide(
			func() string {
				return "Hello, World!"
			},
			func() *zap.Logger {
				return zap.L()
			},
		),
		fx.WithLogger(
			func(logger *zap.Logger) fxevent.Logger {
				return &fxevent.ZapLogger{Logger: logger}
			},
		),
		fx.Invoke(
			func(s string) {
				fmt.Println(s)
			},
		),
	)
	app.Run()
}

When I ran it and pressed ctrl+C, the program successfully terminates with SIGINT (2).

C:\Users\sungy\go\src\github.com\sywhang\playground>go run main.go
Hello, World!
exit status 2
C:\Users\sungy\go\src\github.com\sywhang\playground>

Do you mind quickly verifying whether this program encounters the same issue on your machines?

mpe85 commented

@sywhang There you go:

main.go:

package main

import (
	"context"
	"fmt"
	"go.uber.org/fx"
)

func main() {
	app := fx.New(
		fx.Invoke(func(lifecycle fx.Lifecycle) {
			lifecycle.Append(
				fx.Hook{
					OnStart: func(ctx context.Context) error {
						fmt.Println("OnStart")
						return nil
					},
					OnStop: func(ctx context.Context) error {
						fmt.Println("OnStop")
						return nil
					},
				})
		}),
	)
	app.Run()
}

go.mod:

module go-playground

go 1.17

require go.uber.org/fx v1.15.0

require (
	go.uber.org/atomic v1.6.0 // indirect
	go.uber.org/dig v1.12.0 // indirect
	go.uber.org/multierr v1.5.0 // indirect
	go.uber.org/zap v1.16.0 // indirect
	golang.org/x/sys v0.0.0-20210903071746-97244b99971b // indirect
)

Output:

PS C:\Users\***\go\src\go-playground> go run .
[Fx] PROVIDE    fx.Lifecycle <= go.uber.org/fx.New.func1()
[Fx] PROVIDE    fx.Shutdowner <= go.uber.org/fx.(*App).shutdowner-fm()
[Fx] PROVIDE    fx.DotGraph <= go.uber.org/fx.(*App).dotGraph-fm()
[Fx] INVOKE             main.main.func1()
[Fx] HOOK OnStart               main.main.func1.1() executing (caller: main.main.func1)
OnStart
[Fx] HOOK OnStart               main.main.func1.1() called by main.main.func1 ran successfully in 550.5µs
[Fx] RUNNING
exit status 0xc000013a

When prssing CTRL+C it exits with message exit status 0xc000013a and does not call the OnStop lifecycle hook.

  • 👍2
wesom commented

@Arekusei I believe package syscall is deprecated and that is why we are using golang.org/x/sys/windows and golang.org/x/sys/unix instead:

Deprecated: this package is locked down. Callers should use the corresponding package in the golang.org/x/sys repository instead

@mpe85 @wesom do you have a minimal repro for this issue? I just tried to repro this on my Windows box and it seems to be working fine.

Here's the sample hello world program I used:

package main

import (
	"fmt"

	"go.uber.org/fx"
	"go.uber.org/fx/fxevent"
	"go.uber.org/zap"
)

func main() {
	app := fx.New(
		fx.Provide(
			func() string {
				return "Hello, World!"
			},
			func() *zap.Logger {
				return zap.L()
			},
		),
		fx.WithLogger(
			func(logger *zap.Logger) fxevent.Logger {
				return &fxevent.ZapLogger{Logger: logger}
			},
		),
		fx.Invoke(
			func(s string) {
				fmt.Println(s)
			},
		),
	)
	app.Run()
}

When I ran it and pressed ctrl+C, the program successfully terminates with SIGINT (2).

C:\Users\sungy\go\src\github.com\sywhang\playground>go run main.go
Hello, World!
exit status 2
C:\Users\sungy\go\src\github.com\sywhang\playground>

Do you mind quickly verifying whether this program encounters the same issue on your machines?

The program exit with message exit status 0xc000013a on my win10.

Arekusei commented

@sywhang oh sorry. I didn't see this deprecation.
If project will continue using x/sys/windows package then there is only one solution that can help- cast to syscall.Signal type:

signal.Notify(sig, syscall.Signal(windows.SIGINT), syscall.Signal(windows.SIGTERM))

But again, it uses syscall package from std.

If you look into implementations of similar constants in unix and windows packages
unix: https://cs.opensource.google/go/x/sys/+/master:unix/zerrors_linux.go;l=2993
windows: https://cs.opensource.google/go/x/sys/+/master:windows/types_windows.go;l=33
You will see that windows package uses its own type while unix package uses syscall.Signal.
And when we look into implementation of Notify function we will see next:
https://github.com/golang/go/blob/master/src/os/signal/signal.go#L165
https://github.com/golang/go/blob/f6647f2e3bc0b803a67c97a7d5d8733cefbd5d5b/src/os/signal/signal_unix.go#L35

Notify requires syscall.Signal type to listen signals correctly.

And probably it's needed to report issue into golang repository about this.

sywhang commented

@mpe85 thanks for the repro - I repro'd it locally and I think I now got a working fix locally:

C:\Users\sungy\go\src\github.com\sywhang\playground>go run main.go
[Fx] PROVIDE    fx.Lifecycle <= go.uber.org/fx.New.func1()
[Fx] PROVIDE    fx.Shutdowner <= go.uber.org/fx.(*App).shutdowner-fm()
[Fx] PROVIDE    fx.DotGraph <= go.uber.org/fx.(*App).dotGraph-fm()
[Fx] INVOKE             main.main.func1()
[Fx] HOOK OnStart               main.main.func1.1() executing (caller: main.main.func1)
OnStart
[Fx] HOOK OnStart               main.main.func1.1() called by main.main.func1 ran successfully in 0s
[Fx] RUNNING
[Fx] INTERRUPT
[Fx] HOOK OnStop                main.main.func1.2() executing (caller: main.main.func1)
OnStop
[Fx] HOOK OnStop                main.main.func1.2() called by main.main.func1 ran successfully in 999.3µs

Does this match what you were expecting?

mpe85 commented

@sywhang Yes, this is what I would expect!