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

Jump to bottom

runtime: use tgkill instead of tkill to keep Android happy #24924

Closed
zx2c4 opened this issue Apr 18, 2018 · 3 comments
Closed

runtime: use tgkill instead of tkill to keep Android happy #24924

zx2c4 opened this issue Apr 18, 2018 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.11

Comments

@zx2c4
Copy link
Contributor

zx2c4 commented Apr 18, 2018

Go uses tkill in the panic routine. From the tkill man page:

   tkill() is an obsolete predecessor to
   tgkill().   It allows only the target
   thread ID to be specified, which  may
   result in the wrong thread being sig‐
   naled if a thread terminates and  its
   thread  ID  is recycled.  Avoid using
   this system call.

Android sets a seccomp filter that disallows tkill but allows tgkill. Therefore the solution is to switch Go to using tgkill. According to the man page, there are other good reasons for switching to tgkill too.

This is similar to #23750 but is yet-another mole.
@ianlancetaylor ianlancetaylor added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Apr 18, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Apr 18, 2018
@ianlancetaylor ianlancetaylor changed the title Crash on Android: tkill isn't allowed by seccomp runtime: use tgkill instead of tkill to keep Android happy Apr 18, 2018
@ianlancetaylor
Copy link
Contributor

CC @eliasnaur

@eliasnaur
Copy link
Contributor

I have no objection to tgkill if it's better, but I do see tkill listed in the seccomp list from 23750:

https://android.googlesource.com/platform/bionic/+/master/libc/seccomp/arm64_app_policy.cpp

Do you have a small test case that breaks Android O?

@zx2c4
Copy link
Contributor Author

zx2c4 commented Apr 18, 2018

Woah, actually, no, I can't reproduce in any of the emulators on any of the architectures. I sent this based on crash logs from some users who might be running strange/old/weirdvendor devices.

So, fine to close this issue.

Sorry for the noise.

@zx2c4 zx2c4 closed this as completed Apr 18, 2018
@prattmic prattmic mentioned this issue Sep 7, 2018
Closed
@golang golang locked and limited conversation to collaborators Apr 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
4 participants
@zx2c4 @eliasnaur @ianlancetaylor @gopherbot