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

x/mobile: when a panic happens, a long enough traceback deadlocks the runtime on Android #35590

Open
gawen opened this issue Nov 14, 2019 · 7 comments · May be fixed by golang/mobile#40
Open
Labels
mobile Android, iOS, and x/mobile NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@gawen
Copy link
Contributor

gawen commented Nov 14, 2019

What version of Go are you using (go version)?

$ go version
go version go1.13.1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home//.cache/go-build"
GOENV="/home//.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/opt/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build557544076=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I have an Go application compiled for Android with gomobile. When a panic happens, the runtime prints a traceback and quits. However, when this traceback is long enough, the application freezes and never quits.

Gomobile redirects stderr to Android (Android log tag being "GoLog"). When a panic happens, the world is frozen, so:

Disabling the gomobile's redirection from stderr makes the application crashes "properly".

Note that the runtime already redirect logs to Android (with the tag "Go"), so the redirection of stderr by gomobile seems redundant to me?

To reproduce, you can modify the runtime to println lot of lines when a panic is caught, until the runtime freezes.

What did you expect to see?

When a panic happens, and the resulting traceback is long enough, I expect the runtime to print the full traceback and quit.

What did you see instead?

When a panic happens, and the resulting traceback is long enough, I see the runtime prints a truncated traceback and freezes.

@steeve
Copy link
Contributor

steeve commented Nov 14, 2019

cc @eliasnaur, I know you don't maintain it anymore but it would be nice to know if the now built-in support in Go itself would enable to remove this bit in gomobile, or there were some other reason for it to exist?

@gawen gawen changed the title mobile: when a panic happens, a long enough traceback deadlocks the runtime on Android x/mobile: when a panic happens, a long enough traceback deadlocks the runtime on Android Nov 14, 2019
@gopherbot gopherbot added this to the Unreleased milestone Nov 14, 2019
@gopherbot gopherbot added the mobile Android, iOS, and x/mobile label Nov 14, 2019
@eliasnaur
Copy link
Contributor

AFAICT, the runtime only redirects its own printing (of crashes) to stderr. The gomobile redirect is for all writes to stderr (and stdout), not just the runtime's.

One fix is to rewrite the gomobile redirect in C(go).

@steeve
Copy link
Contributor

steeve commented Nov 15, 2019

@steeve
Copy link
Contributor

steeve commented Nov 15, 2019

Another idea would be to implement a buffered writer and only replace os.Stdout/Stderr. That won't redirect all stdout/stderr logging, but Go code would be fine, and stderr wouldn't be duped, so the dump would work.

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 15, 2019
@hyangah
Copy link
Contributor

hyangah commented Nov 18, 2019

Wonder if it's possible to let the gomobile directly send all logs to logd using the code similar to runtime/write_err_android.go. More desirably, would be nice if it allows users to turn it on and off - I found logging from some libraries is too chatty for mobile application use cases.

gawen pushed a commit to gawen/mobile that referenced this issue Dec 20, 2019
As stderr may be written while the world is frozen (e.g. the runtime
dumping the traceback of a panic), the redirection of stdout and stderr
to Android's logcat cannot be done in Go but in C.

This implementation spawns a detached thread which will wait for stdout
or stderr to be readable, and when so, read one line at a time and
write it to Android's logcat.

Fixes golang/go#35590
@gawen
Copy link
Contributor Author

gawen commented Dec 20, 2019

Proposal patch for this issue: golang/mobile#40

@gopherbot
Copy link

Change https://golang.org/cl/212839 mentions this issue: internal/mobileinit: redirect std{out,err} without using Go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mobile Android, iOS, and x/mobile NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants