Ignore SIGURG on Unix (including Darwin)

[djs55]

- What I did

Ignored spurious SIGURG signals generated by the Go 1.14+ runtime on Darwin.

Running the example in golang/go#37942
I see lots of these signals:

dave@m1 sigurg % uname -ms
Darwin arm64
dave@m1 sigurg % go run main.go
received urgent I/O condition: 2021-05-21 16:03:03.482211 +0100 BST m=+0.014553751
received urgent I/O condition: 2021-05-21 16:03:03.507171 +0100 BST m=+0.039514459

Previously I would also see a spurious error when closing a connection:

dave@m1 cli % ./build/docker run -it --init ubuntu
root@7f9fc6aee300:/# exit
Unsupported signal: <nil>. Discarding.

- How I did it

#2929 already covered SIGURG on Linux. I extended it to cover all Unix versions via _unix.go.

I added an ok to detect the signals channel being closed and return from the forwarding function.

- How to verify it

The unit test for Linux should also cover Mac. On Docker Desktop if you run an interactive container:

dave@m1 cli % ./build/docker run -it --init ubuntu

and then watch the Docker API proxy log:

tail -f ~/Library/Containers/com.docker.docker/Data/log/host/com.docker.driver.amd64-linux.log

and observe no

2021-05-21 18:01:39.972896+0900 localhost com.docker.driver.amd64-linux[87081]: proxy >> POST /v1.41/containers/b6e4ab5bb7d4271bae2ad14dfce139f9b0e84f315ae6e9fdac8224cd93c0b780/kill?signal=URG

as seen on docker/for-mac#5712

- Description for the changelog

• Don't inject SIGURG signals generated by the Go runtime into containers

- A picture of a cute animal (not mandatory but encouraged)

[cpuguy83]

Wow, that's surprising. Confirmed this is getting fired off quite rapidly on darwin.

[cpuguy83]

Maybe signals_unix.go and make it !windows
I saw some issues related to bsd in the go repo about this as well.

It's a pretty good bet no one will want to forward this signal, ever.

[thaJeztah]

Was wondering if we should filter by name; does windows also have a "SIGURG" defined?

[djs55]

I've pushed an update to use signals_unix.go and !windows. Hopefully CircleCI's infra is back on line!

It looks like SIGURG isn't defined on Windows, according to https://stackoverflow.com/a/63396672/6774019 . According to the x/sys/windows docs there is only

const (
// More invented values for signalsSIGHUP=Signal(0x1)
SIGINT=Signal(0x2)
SIGQUIT=Signal(0x3)
SIGILL=Signal(0x4)
SIGTRAP=Signal(0x5)
SIGABRT=Signal(0x6)
SIGBUS=Signal(0x7)
SIGFPE=Signal(0x8)
SIGKILL=Signal(0x9)
SIGSEGV=Signal(0xb)
SIGPIPE=Signal(0xd)
SIGALRM=Signal(0xe)
SIGTERM=Signal(0xf)
)

I guess this is good news 😅

[codecov-commenter]

Codecov Report

Merging #3103 (18466a2) into master (4623696) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 18466a2 differs from pull request most recent head cedaf44. Consider uploading reports for the commit cedaf44 to get more accurate results

@@ Coverage Diff @@## master #3103 +/- ##
==========================================
- Coverage 56.97% 56.96% -0.01% 
==========================================
Files 299 299 Lines 18729 18732 +3 ==========================================
Hits 10670 10670 - Misses 7192 7194 +2 - Partials 867 868 +1 

[cpuguy83]

Build tags are redundant here. _windows implicitly makes this only build on windows.

[djs55]

Good point! I've repushed, removing the build tags here.

[cpuguy83]

Sorry usually this is !windows. Any reason to restrict this from other unixes?

[djs55]

Ah, no reason! I agree having signals_windows.go and then a signals_unix.go with !windows makes sense. For such a small patch it's taking me a while to get it right :)

[cpuguy83]

LGTM

Thanks for dealing with nitpicks.

[thaJeztah]

Second commit looks to be the same as #2986 (which removes the warning as well); let me check if that one can be merged 🤗

[djs55]

I've force-pushed to remove the second commit, since it has been independently fixed in #2986

[thaJeztah]

LGTM, thanks!