From 46f9e01d1e1beffc7ae6b99389c6e7e7ceb9c490 Mon Sep 17 00:00:00 2001 From: Elias Naur Date: Mon, 8 Feb 2016 01:02:32 +0100 Subject: [PATCH] mobile/bind: ensure that Java->Go->Java calls stay on same thread Java methods from Go are run on a thread pool managed on the Java side, to avoid the complexities of getting Go threads to play along with the Android JVM. However, for call stacks that contain a Java->Go->Java chain, this behaviour confuses Java code sensitive to specific threads if the Go->Java call is executed on an arbitrary thread from the pool. For example, most Android UI changes must happen on the single UI thread. Replace the thread pool with direct calls to mimic ObjC<->Go and Java->Go calls. Threads not already attached to the JVM are attached. Introduce a thread local variable to detach such threads at thread exit. Change-Id: I8cb65803c9278666ae77a0c7a65dc2d9c7e739e1 Reviewed-on: https://go-review.googlesource.com/19334 Reviewed-by: David Crawshaw --- bind/java/Seq.java | 57 ++++-------------- bind/java/seq_android.c | 126 ++++++++++++++++++++++++++++++--------- bind/java/seq_android.go | 101 ++++++------------------------- bind/java/seq_android.h | 2 + 4 files changed, 130 insertions(+), 156 deletions(-) diff --git a/bind/java/Seq.java b/bind/java/Seq.java index b7cfa6b..2e8879a 100644 --- a/bind/java/Seq.java +++ b/bind/java/Seq.java @@ -5,8 +5,6 @@ package go; import java.util.Arrays; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.logging.Logger; // Seq is a sequence of machine-dependent encoded values. @@ -30,9 +28,6 @@ public class Seq { } initSeq(); - new Thread("GoSeq") { - public void run() { Seq.receive(); } - }.start(); } @SuppressWarnings("UnusedDeclaration") @@ -108,55 +103,23 @@ public class Seq { // a Ref to the receiver. public static native void send(String descriptor, int code, Seq src, Seq dst); - // recv returns the next request from Go for a Java call. - static native void recv(Seq in, Receive params); - - // recvRes sends the result of a Java call back to Go. - static native void recvRes(int handle, Seq out); - - static final class Receive { - int refnum; - int code; - int handle; - } - protected void finalize() throws Throwable { super.finalize(); free(); } private native void free(); - private static final ExecutorService receivePool = Executors.newCachedThreadPool(); - - // receive listens for callback requests from Go, invokes them on a thread - // pool and sends the responses. - public static void receive() { - Seq.Receive params = new Seq.Receive(); - while (true) { - final Seq in = new Seq(); - Seq.recv(in, params); - - final int code = params.code; - final int handle = params.handle; - final int refnum = params.refnum; - - if (code == -1) { - // Special signal from seq.FinalizeRef. - tracker.dec(refnum); - Seq out = new Seq(); - Seq.recvRes(handle, out); - continue; - } - - receivePool.execute(new Runnable() { - public void run() { - Ref r = tracker.get(refnum); - Seq out = new Seq(); - r.obj.call(code, in, out); - Seq.recvRes(handle, out); - } - }); + public static Seq recv(Seq in, int code, int refnum) { + Seq out = new Seq(); + if (code == -1) { + // Special signal from seq.FinalizeRef. + tracker.dec(refnum); + return out; } + + Ref r = tracker.get(refnum); + r.obj.call(code, in, out); + return out; } // An Object is a Java object that matches a Go object. diff --git a/bind/java/seq_android.c b/bind/java/seq_android.c index 78e1030..4a72113 100644 --- a/bind/java/seq_android.c +++ b/bind/java/seq_android.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "seq_android.h" #include "_cgo_export.h" @@ -15,12 +16,19 @@ #define LOG_FATAL(...) __android_log_print(ANDROID_LOG_FATAL, "go/Seq", __VA_ARGS__) static jfieldID memptr_id; -static jfieldID receive_refnum_id; -static jfieldID receive_code_id; -static jfieldID receive_handle_id; static jclass jbytearray_clazz; +static jclass seq_clazz; +static jmethodID seq_cons; +static jmethodID seq_recv; + +static JavaVM *jvm; +// jnienvs holds the per-thread JNIEnv* for Go threads where we called AttachCurrentThread. +// A pthread key destructor is supplied to call DetachCurrentThread on exit. This trick is +// documented in http://developer.android.com/training/articles/perf-jni.html under "Threads". +static pthread_key_t jnienvs; + // pinned represents a pinned array to be released at the end of Send call. typedef struct pinned { jobject ref; @@ -201,15 +209,100 @@ static jclass find_class(JNIEnv *env, const char *class_name) { return (*env)->NewGlobalRef(env, clazz); } +static jmethodID get_method_id(JNIEnv *env, jclass clazz, const char *name, const char *sig) { + jmethodID m = (*env)->GetMethodID(env, clazz, name, sig); + if (m == NULL) { + describe_exception(env); + LOG_FATAL("cannot find method %s", name); + } + return m; +} + +static jmethodID get_static_method_id(JNIEnv *env, jclass clazz, const char *name, const char *sig) { + jmethodID m = (*env)->GetStaticMethodID(env, clazz, name, sig); + if (m == NULL) { + describe_exception(env); + LOG_FATAL("cannot find static method %s", name); + } + return m; +} + +void recv(int32_t ref, int code, uint8_t *in_ptr, size_t in_len, uint8_t **out_ptr, size_t *out_len) { + jobject out; + mem *out_mem; + mem *in_mem; + JNIEnv *env; + jobject in; + jint ret; + + ret = (*jvm)->GetEnv(jvm, (void **)&env, JNI_VERSION_1_6); + if (ret != JNI_OK) { + if (ret != JNI_EDETACHED) { + LOG_FATAL("failed to get thread env"); + return; + } + if ((*jvm)->AttachCurrentThread(jvm, &env, NULL) != JNI_OK) { + LOG_FATAL("failed to attach current thread"); + return; + } + pthread_setspecific(jnienvs, env); + } + + in = (*env)->NewObject(env, seq_clazz, seq_cons); + if (in == NULL) { + describe_exception(env); + LOG_FATAL("cannot instantiate Seq"); + return; + } + in_mem = mem_get(env, in); + if (in_mem == NULL) { + LOG_FATAL("recv on NULL in_mem"); + return; + } + memcpy(mem_write(env, in, in_len, 1), in_ptr, in_len); + in_mem->off = 0; + out = (*env)->CallStaticObjectMethod(env, seq_clazz, seq_recv, in, code, ref); + if (out == NULL) { + describe_exception(env); + LOG_FATAL("failed to invoke Seq.recv"); + return; + } + out_mem = mem_get(env, out); + if (out_mem == NULL) { + LOG_FATAL("recv on NULL out_mem"); + return; + } + *out_ptr = out_mem->buf; + *out_len = out_mem->len; +} + +// env_destructor is registered as a thread data key destructor to +// clean up a Go thread that is attached to the JVM. +static void env_destructor(void *env) { + if ((*jvm)->DetachCurrentThread(jvm) != JNI_OK) { + LOG_INFO("failed to detach current thread"); + } +} + JNIEXPORT void JNICALL Java_go_Seq_initSeq(JNIEnv *env, jclass clazz) { + seq_clazz = (*env)->NewGlobalRef(env, clazz); + seq_recv = get_static_method_id(env, seq_clazz, "recv", "(Lgo/Seq;II)Lgo/Seq;"); + seq_cons = get_method_id(env, seq_clazz, "", "()V"); + memptr_id = find_field(env, "go/Seq", "memptr", "J"); - receive_refnum_id = find_field(env, "go/Seq$Receive", "refnum", "I"); - receive_handle_id = find_field(env, "go/Seq$Receive", "handle", "I"); - receive_code_id = find_field(env, "go/Seq$Receive", "code", "I"); jclass bclazz = find_class(env, "[B"); jbytearray_clazz = (*env)->NewGlobalRef(env, bclazz); + + if ((*env)->GetJavaVM(env, &jvm) != 0) { + LOG_FATAL("failed to get JVM"); + return; + } + if (pthread_key_create(&jnienvs, env_destructor) != 0) { + LOG_FATAL("failed to initialize jnienvs thread local storage"); + return; + } } JNIEXPORT void JNICALL @@ -434,27 +527,6 @@ Java_go_Seq_send(JNIEnv *env, jclass clazz, jstring descriptor, jint code, jobje } } -JNIEXPORT void JNICALL -Java_go_Seq_recv(JNIEnv *env, jclass clazz, jobject in_obj, jobject receive) { - mem *in = mem_get(env, in_obj); - if (in == NULL) { - LOG_FATAL("recv in is NULL"); - } - struct Recv_return ret = Recv(&in->buf, &in->len); - (*env)->SetIntField(env, receive, receive_refnum_id, ret.r0); - (*env)->SetIntField(env, receive, receive_code_id, ret.r1); - (*env)->SetIntField(env, receive, receive_handle_id, ret.r2); -} - -JNIEXPORT void JNICALL -Java_go_Seq_recvRes(JNIEnv *env, jclass clazz, jint handle, jobject out_obj) { - mem *out = mem_get(env, out_obj); - if (out == NULL) { - LOG_FATAL("recvRes out is NULL"); - } - RecvRes((int32_t)handle, out->buf, out->len); -} - JNIEXPORT void JNICALL Java_go_Seq_setContext(JNIEnv* env, jclass clazz, jobject ctx) { JavaVM* vm; diff --git a/bind/java/seq_android.go b/bind/java/seq_android.go index 9498e38..a69362b 100644 --- a/bind/java/seq_android.go +++ b/bind/java/seq_android.go @@ -13,7 +13,6 @@ package java // import "golang.org/x/mobile/bind/java" import "C" import ( "fmt" - "sync" "unsafe" "golang.org/x/mobile/bind/seq" @@ -59,34 +58,6 @@ func DestroyRef(refnum C.int32_t) { seq.Delete(int32(refnum)) } -type request struct { - ref *seq.Ref - handle int32 - code int - in *seq.Buffer -} - -var recv struct { - sync.Mutex - cond sync.Cond // signals req is not empty - req []request - next int32 // next handle value -} - -var res struct { - sync.Mutex - cond sync.Cond // signals a response is filled in - out map[int32]*seq.Buffer // handle -> output -} - -func init() { - recv.cond.L = &recv.Mutex - recv.next = 411 // arbitrary starting point distinct from Go and Java obj ref nums - - res.cond.L = &res.Mutex - res.out = make(map[int32]*seq.Buffer) -} - func seqToBuf(bufptr **C.uint8_t, lenptr *C.size_t, buf *seq.Buffer) { if debug { fmt.Printf("seqToBuf tag 1, len(buf.Data)=%d, *lenptr=%d\n", len(buf.Data), *lenptr) @@ -108,64 +79,30 @@ func seqToBuf(bufptr **C.uint8_t, lenptr *C.size_t, buf *seq.Buffer) { C.memcpy(unsafe.Pointer(*bufptr), unsafe.Pointer(&buf.Data[0]), C.size_t(len(buf.Data))) } -// Recv is called by Java in a loop and blocks until Go requests a callback -// be executed by the JVM. Then a request object is returned, along with a -// handle for the host to respond via RecvRes. -//export Recv -func Recv(in **C.uint8_t, inlen *C.size_t) (ref, code, handle C.int32_t) { - recv.Lock() - for len(recv.req) == 0 { - recv.cond.Wait() - } - req := recv.req[0] - recv.req = recv.req[1:] - seqToBuf(in, inlen, req.in) - recv.Unlock() - - return C.int32_t(req.ref.Num), C.int32_t(req.code), C.int32_t(req.handle) -} - -// RecvRes is called by JNI to return the result of a requested callback. -//export RecvRes -func RecvRes(handle C.int32_t, out *C.uint8_t, outlen C.size_t) { - outBuf := &seq.Buffer{ - Data: make([]byte, outlen), - } - copy(outBuf.Data, (*[maxSliceLen]byte)(unsafe.Pointer(out))[:outlen]) - - res.Lock() - res.out[int32(handle)] = outBuf - res.Unlock() - res.cond.Broadcast() -} - // transact calls a method on a Java object instance. // It blocks until the call is complete. -func transact(ref *seq.Ref, _ string, code int, in *seq.Buffer) *seq.Buffer { - recv.Lock() - if recv.next == 1<<31-1 { - panic("recv handle overflow") - } - handle := recv.next - recv.next++ - recv.req = append(recv.req, request{ - ref: ref, - code: code, - in: in, - handle: handle, - }) - recv.Unlock() - recv.cond.Signal() +func transact(ref *seq.Ref, _ string, code int, inBuf *seq.Buffer) *seq.Buffer { + var ( + out *C.uint8_t = nil + outLen C.size_t = 0 + in *C.uint8_t = nil + inLen C.size_t = 0 + ) - res.Lock() - for res.out[handle] == nil { - res.cond.Wait() + if len(inBuf.Data) > 0 { + in = (*C.uint8_t)(unsafe.Pointer(&inBuf.Data[0])) + inLen = C.size_t(len(inBuf.Data)) } - out := res.out[handle] - delete(res.out, handle) - res.Unlock() - return out + C.recv(C.int32_t(ref.Num), C.int(code), in, inLen, &out, &outLen) + if outLen > 0 { + outBuf := &seq.Buffer{ + Data: make([]byte, outLen), + } + copy(outBuf.Data, (*[maxSliceLen]byte)(unsafe.Pointer(out))[:outLen]) + return outBuf + } + return nil } func encodeString(out *seq.Buffer, v string) { diff --git a/bind/java/seq_android.h b/bind/java/seq_android.h index 5ae09c5..5435baa 100644 --- a/bind/java/seq_android.h +++ b/bind/java/seq_android.h @@ -3,3 +3,5 @@ // license that can be found in the LICENSE file. void init_seq(void* vm, void* classfinder); +JNIEnv *get_thread_env(void); +void recv(int32_t ref, int code, uint8_t *in_ptr, size_t in_len, uint8_t **out_ptr, size_t *out_len);