Skip to content
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/signal: timeout in TestAllThreadsSyscallSignals [1.16 backport] #45307

Closed
gopherbot opened this issue Mar 30, 2021 · 9 comments
Closed

os/signal: timeout in TestAllThreadsSyscallSignals [1.16 backport] #45307

gopherbot opened this issue Mar 30, 2021 · 9 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone

Comments

@gopherbot
Copy link

@ianlancetaylor requested issue #44193 to be considered for backport to the next 1.16 minor release.

@gopherbot Please open backport to 1.16

This bug can cause a rare deadlock if a signal is received during a call to syscall.Setuid or various similar functions on GNU/Linux.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Mar 30, 2021
@gopherbot gopherbot added this to the Go1.16.3 milestone Mar 30, 2021
@toothrot toothrot added the CherryPickApproved Used during the release process for point releases label Apr 1, 2021
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Apr 1, 2021
@toothrot
Copy link
Contributor

toothrot commented Apr 1, 2021

Approved. This is a serious issue with no workaround.

@dmitshur dmitshur modified the milestones: Go1.16.3, Go1.16.4 Apr 1, 2021
@cagedmantis
Copy link
Contributor

@AndrewGMorgan Would it be possible to create the cherry-pick CL before the upcoming release?

@AndrewGMorgan
Copy link
Contributor

Sure, happy to.

I've not done this before. Do I checkout the 1.16 branch and cherry pick to that, or are you asking if it is safe for a robot to just do it?

@ianlancetaylor
Copy link
Contributor

You can cherry pick entirely in Gerrit. See https://golang.org/wiki/MinorReleases.

@gopherbot
Copy link
Author

Change https://golang.org/cl/316869 mentions this issue: [release-branch.go1.16] syscall: syscall.AllThreadsSyscall signal handling fixes

@AndrewGMorgan
Copy link
Contributor

That cherry pick fails to complete ./all.bash for me. It fails with:

--- FAIL: TestDependencies (1.18s)
    deps_test.go:584: unexpected dependency: internal/buildcfg imports [runtime]
FAIL
FAIL    go/build        1.697s

That particular failure, however, does not seem to be related to my cherry pick since it fails with the HEAD^ commit too on this branch. I'll see if I can figure out what introduced the failure.

@AndrewGMorgan
Copy link
Contributor

I guess I must be holding it wrong. For me. this seems to fail for all go1.16 release tags, when my bootstrap is using go1.16.3. I guess all of these release tags can't have been used if this is a general problem. I'll try again with something a lot older.

@AndrewGMorgan
Copy link
Contributor

AndrewGMorgan commented May 4, 2021

For completeness, starting over from scratch works fine:

$ git clone https://go.googlesource.com/go
$ cd go/src/
$ git fetch https://go.googlesource.com/go refs/changes/69/316869/2 && git checkout -b change-316869 FETCH_HEAD
$ ./all.bash 

Perhaps there is something left over from compiling at HEAD that is confusing the all.bash script in this branch? That is, this fails:

$ git clone https://go.googlesource.com/go
$ cd go/src/
$ ./all.bash && git checkout -b test1.16.3 go1.16.3 && ./all.bash

@gopherbot
Copy link
Author

Closed by merging ce04f86 to release-branch.go1.16.

gopherbot pushed a commit that referenced this issue May 4, 2021
…dling fixes

The runtime support for syscall.AllThreadsSyscall() functions had
some corner case deadlock issues when signal handling was in use.
This was observed in at least 3 build test failures on ppc64 and
amd64 architecture CGO_ENABLED=0 builds over the last few months.

The fixes involve more controlled handling of signals while the
AllThreads mechanism is being executed. Further details are
discussed in bug #44193.

The all-threads syscall support is new in go1.16, so earlier
releases are not affected by this bug.

Fixes #45307

Change-Id: I01ba8508a6e1bb2d872751f50da86dd07911a41d
Reviewed-on: https://go-review.googlesource.com/c/go/+/305149
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Pratt <mpratt@google.com>
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
(cherry picked from commit 7e97e4e)
Reviewed-on: https://go-review.googlesource.com/c/go/+/316869
Run-TryBot: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

6 participants