apps/nsqd: fix crash on windows because program.nsqd uninitialized

[supermario1990]

see #1358

[mreiferson]

Thanks for the PR @supermario1990, but I don't actually understand how p.nsqd would be nil? Can you paste logs of the startup/crash on windows?

[supermario1990]

Thanks for the PR @supermario1990, but I don't actually understand how p.nsqd would be nil? Can you paste logs of the startup/crash on windows?

yes, below is logs of the startup/crash on windows:

$ ./nsqd.exe
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x20 pc=0x7930e9]

goroutine 1 [running]:
github.com/nsqio/nsq/nsqd.(*NSQD).Context(...)
        E:/github/nsq/nsqd/nsqd.go:746
main.(*program).Context(0xc000004120, 0x8d1d10, 0xc000004120)
        E:/github/nsq/apps/nsqd/main.go:117 +0x9
github.com/judwhite/go-svc.Run(0x8d1d10, 0xc000004120, 0xc00004f020, 0x1, 0x1, 0x41be01, 0x0)
        F:/code_go/pkg/mod/github.com/judwhite/go-svc@v1.2.1/svc_windows.go:57 +0x2b5
main.main()
        E:/github/nsq/apps/nsqd/main.go:31 +0x9d

[ploxiln]

I think this problem was introduced when adding the program.Context() cancellation feature to go-svc earlier this year. On windows, program.Context() is called before program.Start() (before program.Init() even!) while on unix it is called after.

I think this solution is a bit messy - create a temporary empty NSQD during the "early" program.Context() call, and public setter and getter just to propagate that internal context to the real NSQD later ... and it doesn't really work because that internal cancellable context is not initialized until New().

I think it would be better to do the nsqd.New in program.Init(), and update go-svc to call program.Context() after Init(), at least. Or go back to the drawing board with this nsqd internal exit feature, which has had other growing pains already :)

[ploxiln]

reference: https://github.com/judwhite/go-svc/blob/v1.2.1/svc_windows.go#L57

[mreiferson]

Was just typing exactly what @ploxiln just wrote. Anyway, this is the important bit:

I think it would be better to do the nsqd.New in program.Init(), and update go-svc to call program.Context() after Init(), at least. Or go back to the drawing board with this nsqd internal exit feature, which has had other growing pains already :)

I just sat down to play around with stamping a new release with #1355, and it seems like we need to get this to a good place first.

[mreiferson]

see #1361

[ploxiln]

@supermario1990 we think we fixed this in #1361 (and judwhite/go-svc#28 temporarily being used instead of pure upstream).

Can you confirm that latest master works for you on windows now?

[ploxiln]

I've confirmed that the new nsq-v1.2.1 release works on Windows 10 when run manually/directly in powershell or cmd.exe (with nsqd, nsqlookupd, nsqadmin, nsq_tail). They all start, they communicate, and they exit for CTRL-C. I haven't tested running nsqd etc as a windows service, I'm not up for that :)