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

x/mobile/app: race condition between initSeq and user code #10903

Closed
eliasnaur opened this issue May 18, 2015 · 1 comment
Closed

x/mobile/app: race condition between initSeq and user code #10903

eliasnaur opened this issue May 18, 2015 · 1 comment
Labels
FrozenDueToAge

Comments

@eliasnaur
Copy link
Contributor

app.run(...) in app/android.go closes mainCalled to signal callMain that it can return to call_main_and_wait, which returns to Go.run(), which finally returns from go.Go.init(), letting user code continue. If user code calls anything touching the seq machinery after the closing of mainCalled but before app.stateStart calls java.Init, a crash will happen. One such crash trace is copied in the end of this report.

It suffices to insert a time.Sleep(2*time.Second) just after the close(mainCalled) in app/android.go to allow calls from java into Go and thereby reproduce the bug:

// notifyInitDone informs Java that the program is initialized.
// A NativeActivity will not create a window until this is called.
func run(callbacks []Callbacks) {
    // We want to keep the event loop on a consistent OS thread.
    runtime.LockOSThread()

    ctag := C.CString("Go")
    cstr := C.CString("app.Run")
    C.__android_log_write(C.ANDROID_LOG_INFO, ctag, cstr)
    C.free(unsafe.Pointer(ctag))
    C.free(unsafe.Pointer(cstr))

    close(mainCalled)
    time.Sleep(2 * time.Second) <<<<<<< insert sleep here
    if C.current_native_activity == nil {
        stateStart(callbacks)
        // TODO: stateStop under some conditions.
        select {}
    } else {
        for w := range windowCreated {
            windowDraw(callbacks, w, queue)
        }
    }
}

I have yet to find a satisfactory fix; simply moving the close(mainCalled) just after the stateStart() call results in a panic:

E/Go      (20546): panic: app.GetConfig is not available before app.Run is called
E/Go      (20546): goroutine 7 [running, locked to thread]:
E/Go      (20546): golang.org/x/mobile/app.GetConfig(0x0, 0x0)
E/Go      (20546):  /home/elias/dev/go/src/golang.org/x/mobile/app/app.go:111 +0x108
E/Go      (20546): golang.org/x/mobile/bind/java.initSeq()
E/Go      (20546):  /home/elias/dev/go/src/golang.org/x/mobile/bind/java/seq_android.go:82 +0x1c
E/Go      (20546): golang.org/x/mobile/bind/java.Init()
E/Go      (20546):  /home/elias/dev/go/src/golang.org/x/mobile/bind/java/doc.go:15 +0x1c
E/Go      (20546): golang.org/x/mobile/app.stateStart(0x93adc320, 0x1, 0x1)
E/Go      (20546):  /home/elias/dev/go/src/golang.org/x/mobile/app/state.go:63 +0xe4
E/Go      (20546): golang.org/x/mobile/app.run(0x93adc320, 0x1, 0x1)
E/Go      (20546):  /home/elias/dev/go/src/golang.org/x/mobile/app/android.go:272 +0x114
E/Go      (20546): golang.org/x/mobile/app.Run(0xaf38ecf4, 0x0, 0x0, 0x0, 0x0)
E/Go      (20546):  /home/elias/dev/go/src/golang.org/x/mobile/app/app.go:26 +0xe4
E/Go      (20546): main.main()
E/Go      (20546):  /tmp/gomobile-bind-work-212601191/androidlib/main.go:12 +0x58
E/Go      (20546): golang.org/x/mobile/app/internal/callfn.CallFn(0xaf263428)
E/Go      (20546):  /home/elias/dev/go/src/golang.org/x/mobile/app/internal/callfn/callfn_arm.s:10 +0x20
E/Go      (20546): created by golang.org/x/mobile/app.callMain
E/Go      (20546):  /home/elias/dev/go/src/golang.org/x/mobile/app/android.go:79 +0x16c
E/Go      (20546): goroutine 17 [chan receive, locked to thread]:
E/Go      (20546): golang.org/x/mobile/app.callMain(0xaf263428)
E/Go      (20546):  /home/elias/dev/go/src/golang.org/x/mobile/app/android.go:80 +0x18c
...

Crash trace:

F/art     (21085): art/runtime/check_jni.cc:65] JNI DETECTED ERROR IN APPLICATION: jfieldID was NULL
F/art     (21085): art/runtime/check_jni.cc:65]     in call to GetLongField
F/art     (21085): art/runtime/check_jni.cc:65]     from void go.Seq.ensure(int)
F/art     (21085): art/runtime/check_jni.cc:65] "GoReceive" prio=5 tid=14 Runnable
F/art     (21085): art/runtime/check_jni.cc:65]   | group="main" sCount=0 dsCount=0 obj=0x12c07e20 self=0xb4a35c00
F/art     (21085): art/runtime/check_jni.cc:65]   | sysTid=21121 nice=0 cgrp=default sched=0/0 handle=0xb491cb80
F/art     (21085): art/runtime/check_jni.cc:65]   | state=R schedstat=( 1233439 33281 6 ) utm=0 stm=0 core=2 HZ=100
F/art     (21085): art/runtime/check_jni.cc:65]   | stack=0x835a6000-0x835a8000 stackSize=1036KB
F/art     (21085): art/runtime/check_jni.cc:65]   | held mutexes= "mutator lock"(shared held)
F/art     (21085): art/runtime/check_jni.cc:65]   native: #00 pc 00004e64  /system/lib/libbacktrace_libc++.so (UnwindCurrent::Unwind(unsigned int, ucontext*)+23)
F/art     (21085): art/runtime/check_jni.cc:65]   native: #01 pc 00003665  /system/lib/libbacktrace_libc++.so (Backtrace::Unwind(unsigned int, ucontext*)+8)
F/art     (21085): art/runtime/check_jni.cc:65]   native: #02 pc 00256429  /system/lib/libart.so (art::DumpNativeStack(std::__1::basic_ostream<char, std::__1::char_traits<char> >&, int, char const*, art::mirror::ArtMethod*)+84)
F/art     (21085): art/runtime/check_jni.cc:65]   native: #03 pc 00238fe7  /system/lib/libart.so (art::Thread::Dump(std::__1::basic_ostream<char, std::__1::char_traits<char> >&) const+158)
F/art     (21085): art/runtime/check_jni.cc:65]   native: #04 pc 000b191b  /system/lib/libart.so (art::JniAbort(char const*, char const*)+610)
F/art     (21085): art/runtime/check_jni.cc:65]   native: #05 pc 000b2055  /system/lib/libart.so (art::JniAbortF(char const*, char const*, ...)+68)
F/art     (21085): art/runtime/check_jni.cc:65]   native: #06 pc 000b41c9  /system/lib/libart.so (art::ScopedCheck::CheckInstanceFieldID(_jobject*, _jfieldID*)+588)
F/art     (21085): art/runtime/check_jni.cc:65]   native: #07 pc 000b94ed  /system/lib/libart.so (art::CheckJNI::GetLongField(_JNIEnv*, _jobject*, _jfieldID*)+60)
F/art     (21085): art/runtime/check_jni.cc:65]   native: #08 pc 0006a7f0  /data/app/com.eliasnaur.goreentrant-2/lib/arm/libgojni.so (mem_get+52)
F/art     (21085): art/runtime/check_jni.cc:65]   at go.Seq.ensure(Native method)
F/art     (21085): art/runtime/check_jni.cc:65]   at go.Seq.<init>(Seq.java:21)
F/art     (21085): art/runtime/check_jni.cc:65]   at go.Seq.receive(Seq.java:109)
F/art     (21085): art/runtime/check_jni.cc:65]   at go.Go$1.run(Go.java:30)
...
@gopherbot
Copy link

CL https://golang.org/cl/10296 mentions this issue.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
imWildCat pushed a commit to imWildCat/go-mobile that referenced this issue Apr 10, 2021
@crawshaw
bind/java: initialize seq machinery from Java
2b27d7a 
This ensures that the java bindings are ready before any calls are
made by user code. As a bonus, the JNIEnv* is from the Seq class so I
believe no tricks are required to find the right class loader.

Fixes golang/go#10903.

Change-Id: I33b3b39cef6cc2da36e271de882ba8d26610ea34
Reviewed-on: https://go-review.googlesource.com/10296
Reviewed-by: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
imWildCat pushed a commit to imWildCat/go-mobile that referenced this issue Apr 11, 2021
@crawshaw
bind/java: initialize seq machinery from Java
143c963 
This ensures that the java bindings are ready before any calls are
made by user code. As a bonus, the JNIEnv* is from the Seq class so I
believe no tricks are required to find the right class loader.

Fixes golang/go#10903.

Change-Id: I33b3b39cef6cc2da36e271de882ba8d26610ea34
Reviewed-on: https://go-review.googlesource.com/10296
Reviewed-by: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@eliasnaur @gopherbot