internal/mobileinit: standardize JNIEnv* access

Replace the direct access to JavaVM* and the global android Context
instance with a function responsible for running attached correctly
to the JVM. This saves having to replicate the logic for attaching an
OS thread to the JVM. While here, check for any unhandled Java
exceptions.

This supersedes cl/11812.

Change-Id: Ic9291fe64083d2bd983c4f8e309941b9c47d60c2
Reviewed-on: https://go-review.googlesource.com/14162
Reviewed-by: Nigel Tao <nigeltao@golang.org>
diff --git a/app/android.c b/app/android.c
index 30b4083..c0f57d2 100644
--- a/app/android.c
+++ b/app/android.c
@@ -180,30 +180,6 @@
 	return NULL;
 }
 
-char* attachJNI(JNIEnv** envp) {
-	if (current_vm == NULL) {
-		return "current_vm not set";
-	}
-
-	switch ((*current_vm)->GetEnv(current_vm, (void**)envp, JNI_VERSION_1_6)) {
-	case JNI_OK:
-		return NULL;
-	case JNI_EDETACHED:
-		// AttachCurrentThread is typically paired with a call to
-		// DetachCurrentThread, however attachJNI is called for
-		// the duration of the main function, which exits when the
-		// process exits. We let Unix take care of thread cleanup.
-		if ((*current_vm)->AttachCurrentThread(current_vm, envp, 0) != 0) {
-			return "cannot attach JVM";
-		}
-		return NULL;
-	case JNI_EVERSION:
-		return "bad JNI version";
-	default:
-		return "unknown GetEnv error";
-	}
-}
-
 int32_t getKeyRune(JNIEnv* env, AInputEvent* e) {
 	return (int32_t)(*env)->CallIntMethod(
 		env,
diff --git a/app/android.go b/app/android.go
index 80cce58..0ddbd38 100644
--- a/app/android.go
+++ b/app/android.go
@@ -45,7 +45,6 @@
 char* initEGLDisplay();
 char* createEGLSurface(ANativeWindow* window);
 char* destroyEGLSurface();
-char* attachJNI(JNIEnv**);
 int32_t getKeyRune(JNIEnv* env, AInputEvent* e);
 */
 import "C"
@@ -53,7 +52,6 @@
 	"fmt"
 	"log"
 	"os"
-	"runtime"
 	"time"
 	"unsafe"
 
@@ -248,21 +246,23 @@
 }
 
 func main(f func(App)) {
+	mainUserFn = f
 	// Preserve this OS thread for:
 	//	1. the attached JNI thread
 	//	2. the GL context
-	runtime.LockOSThread()
-
-	// Calls into NativeActivity functions must be made from
-	// a thread attached to the JNI.
-	var env *C.JNIEnv
-	if errStr := C.attachJNI(&env); errStr != nil {
-		log.Fatalf("app: %s", C.GoString(errStr))
+	if err := mobileinit.RunOnJVM(mainUI); err != nil {
+		log.Fatalf("app: %v", err)
 	}
+}
+
+var mainUserFn func(App)
+
+func mainUI(vm, jniEnv, ctx uintptr) error {
+	env := (*C.JNIEnv)(unsafe.Pointer(jniEnv)) // not a Go heap pointer
 
 	donec := make(chan struct{})
 	go func() {
-		f(app{})
+		mainUserFn(app{})
 		close(donec)
 	}()
 
@@ -291,15 +291,14 @@
 		case <-windowCreated:
 		case q = <-inputQueue:
 		case <-donec:
-			return
+			return nil
 		case cfg := <-windowConfigChange:
 			pixelsPerPt = cfg.pixelsPerPt
 			orientation = cfg.orientation
 		case w := <-windowRedrawNeeded:
 			if C.surface == nil {
 				if errStr := C.createEGLSurface(w); errStr != nil {
-					log.Printf("app: %s (%s)", C.GoString(errStr), eglGetError())
-					return
+					return fmt.Errorf("%s (%s)", C.GoString(errStr), eglGetError())
 				}
 			}
 			sendLifecycle(lifecycle.StageFocused)
@@ -318,8 +317,7 @@
 		case <-windowDestroyed:
 			if C.surface != nil {
 				if errStr := C.destroyEGLSurface(); errStr != nil {
-					log.Printf("app: %s (%s)", C.GoString(errStr), eglGetError())
-					return
+					return fmt.Errorf("%s (%s)", C.GoString(errStr), eglGetError())
 				}
 			}
 			C.surface = nil
diff --git a/asset/asset_android.go b/asset/asset_android.go
index 0476baf..e654fba 100644
--- a/asset/asset_android.go
+++ b/asset/asset_android.go
@@ -5,35 +5,18 @@
 package asset
 
 /*
-#cgo LDFLAGS: -llog -landroid
-#include <android/log.h>
+#cgo LDFLAGS: -landroid
 #include <android/asset_manager.h>
 #include <android/asset_manager_jni.h>
 #include <jni.h>
 #include <stdlib.h>
 
-#define LOG_FATAL(...) __android_log_print(ANDROID_LOG_FATAL, "Go/asset", __VA_ARGS__)
-
 // asset_manager is the asset manager of the app.
 AAssetManager* asset_manager;
 
-void asset_manager_init(void* java_vm, void* ctx) {
-	JavaVM* vm = (JavaVM*)(java_vm);
-	JNIEnv* env;
-	int err;
-	int attached = 0;
-
-	err = (*vm)->GetEnv(vm, (void**)&env, JNI_VERSION_1_6);
-	if (err != JNI_OK) {
-		if (err == JNI_EDETACHED) {
-			if ((*vm)->AttachCurrentThread(vm, &env, 0) != 0) {
-				LOG_FATAL("cannot attach JVM");
-			}
-			attached = 1;
-		} else {
-			LOG_FATAL("GetEnv unexpected error: %d", err);
-		}
-	}
+void asset_manager_init(uintptr_t java_vm, uintptr_t jni_env, uintptr_t ctx) {
+	JavaVM* vm = (JavaVM*)java_vm;
+	JNIEnv* env = (JNIEnv*)jni_env;
 
 	// Equivalent to:
 	//	assetManager = ctx.getResources().getAssets();
@@ -47,16 +30,13 @@
 	// Pin the AssetManager and load an AAssetManager from it.
 	am = (*env)->NewGlobalRef(env, am);
 	asset_manager = AAssetManager_fromJava(env, am);
-
-	if (attached) {
-		(*vm)->DetachCurrentThread(vm);
-	}
 }
 */
 import "C"
 import (
 	"fmt"
 	"io"
+	"log"
 	"os"
 	"sync"
 	"unsafe"
@@ -67,8 +47,13 @@
 var assetOnce sync.Once
 
 func assetInit() {
-	ctx := mobileinit.Context{}
-	C.asset_manager_init(ctx.JavaVM(), ctx.AndroidContext())
+	err := mobileinit.RunOnJVM(func(vm, env, ctx uintptr) error {
+		C.asset_manager_init(C.uintptr_t(vm), C.uintptr_t(env), C.uintptr_t(ctx))
+		return nil
+	})
+	if err != nil {
+		log.Fatalf("asset: %v", err)
+	}
 }
 
 func openAsset(name string) (File, error) {
diff --git a/exp/audio/al/al_android.go b/exp/audio/al/al_android.go
index a076a9c..1113562 100644
--- a/exp/audio/al/al_android.go
+++ b/exp/audio/al/al_android.go
@@ -13,30 +13,9 @@
 #include <AL/al.h>
 #include <AL/alc.h>
 
-typedef enum {
-  AL_INIT_RESULT_OK,
-  AL_INIT_RESULT_CANNOT_ATTACH_JVM,
-  AL_INIT_RESULT_BAD_JNI_VERSION,
-  AL_INIT_RESULT_CANNOT_LOAD_SO
-} ALInitResult;
-
-ALInitResult al_init(void* vm, void* context, void** handle) {
-  JavaVM* current_vm = (JavaVM*)(vm);
-  JNIEnv* env;
-
-  int attached = 0;
-  switch ((*current_vm)->GetEnv(current_vm, (void**)&env, JNI_VERSION_1_6)) {
-  case JNI_OK:
-    break;
-  case JNI_EDETACHED:
-    if ((*current_vm)->AttachCurrentThread(current_vm, &env, 0) != 0) {
-      return AL_INIT_RESULT_CANNOT_ATTACH_JVM;
-    }
-    attached = 1;
-    break;
-  case JNI_EVERSION:
-    return AL_INIT_RESULT_BAD_JNI_VERSION;
-  }
+void al_init(uintptr_t java_vm, uintptr_t jni_env, uintptr_t context, void** handle) {
+  JavaVM* vm = (JavaVM*)java_vm;
+  JNIEnv* env = (JNIEnv*)jni_env;
 
   jclass android_content_Context = (*env)->FindClass(env, "android/content/Context");
   jmethodID get_package_name = (*env)->GetMethodID(env, android_content_Context, "getPackageName", "()Ljava/lang/String;");
@@ -48,13 +27,6 @@
   strlcat(lib_path, "/lib/libopenal.so", sizeof(lib_path));
   *handle = dlopen(lib_path, RTLD_LAZY);
   (*env)->ReleaseStringUTFChars(env, package_name, cpackage_name);
-  if (attached) {
-    (*current_vm)->DetachCurrentThread(current_vm);
-  }
-  if (!*handle) {
-    return AL_INIT_RESULT_CANNOT_LOAD_SO;
-  }
-  return AL_INIT_RESULT_OK;
 }
 
 void call_alEnable(LPALENABLE fn, ALenum capability) {
@@ -195,6 +167,7 @@
 */
 import "C"
 import (
+	"errors"
 	"log"
 	"unsafe"
 
@@ -247,18 +220,15 @@
 )
 
 func initAL() {
-	ctx := mobileinit.Context{}
-	switch C.al_init(ctx.JavaVM(), ctx.AndroidContext(), &alHandle) {
-	case C.AL_INIT_RESULT_OK:
-		// No-op.
-	case C.AL_INIT_RESULT_CANNOT_ATTACH_JVM:
-		log.Fatal("al: cannot attach JVM")
-	case C.AL_INIT_RESULT_BAD_JNI_VERSION:
-		log.Fatal("al: bad JNI version")
-	case C.AL_INIT_RESULT_CANNOT_LOAD_SO:
-		log.Fatal("al: cannot load libopenal.so")
-	default:
-		log.Fatal("al: cannot initialize OpenAL library")
+	err := mobileinit.RunOnJVM(func(vm, env, ctx uintptr) error {
+		C.al_init(C.uintptr_t(vm), C.uintptr_t(env), C.uintptr_t(ctx), &alHandle)
+		if alHandle == nil {
+			return errors.New("al: cannot load libopenal.so")
+		}
+		return nil
+	})
+	if err != nil {
+		log.Fatalf("al: %v", err)
 	}
 
 	alEnableFunc = C.LPALENABLE(fn("alEnable"))
diff --git a/internal/mobileinit/ctx_android.go b/internal/mobileinit/ctx_android.go
index 753a2c8..559f918 100644
--- a/internal/mobileinit/ctx_android.go
+++ b/internal/mobileinit/ctx_android.go
@@ -19,43 +19,106 @@
 // current_ctx is Android's android.context.Context. May be NULL.
 jobject current_ctx;
 
-// Set current_vm and current_ctx. The ctx passed in must be a global
-// reference instance.
-void set_vm_ctx(JavaVM* vm, jobject ctx) {
-	current_vm = vm;
-	current_ctx = ctx;
-	// TODO: check leak
+char* lockJNI(uintptr_t* envp, int* attachedp) {
+	JNIEnv* env;
+
+	if (current_vm == NULL) {
+		return "no current JVM";
+	}
+
+	*attachedp = 0;
+	switch ((*current_vm)->GetEnv(current_vm, (void**)&env, JNI_VERSION_1_6)) {
+	case JNI_OK:
+		break;
+	case JNI_EDETACHED:
+		if ((*current_vm)->AttachCurrentThread(current_vm, &env, 0) != 0) {
+			return "cannot attach to JVM";
+		}
+		*attachedp = 1;
+		break;
+	case JNI_EVERSION:
+		return "bad JNI version";
+	default:
+		return "unknown JNI error from GetEnv";
+	}
+
+	*envp = (uintptr_t)env;
+	return NULL;
+}
+
+char* checkException(uintptr_t jnienv) {
+	jthrowable exc;
+	JNIEnv* env = (JNIEnv*)jnienv;
+
+	if (!(*env)->ExceptionCheck(env)) {
+		return NULL;
+	}
+
+	exc = (*env)->ExceptionOccurred(env);
+	(*env)->ExceptionClear(env);
+
+	jclass clazz = (*env)->FindClass(env, "java/lang/Throwable");
+	jmethodID toString = (*env)->GetMethodID(env, clazz, "toString", "()Ljava/lang/String;");
+	jobject msgStr = (*env)->CallObjectMethod(env, exc, toString);
+	return (char*)(*env)->GetStringUTFChars(env, msgStr, 0);
+}
+
+void unlockJNI() {
+	(*current_vm)->DetachCurrentThread(current_vm);
 }
 */
 import "C"
 
-import "unsafe"
+import (
+	"errors"
+	"runtime"
+	"unsafe"
+)
 
 // SetCurrentContext populates the global Context object with the specified
 // current JavaVM instance (vm) and android.context.Context object (ctx).
 // The android.context.Context object must be a global reference.
 func SetCurrentContext(vm, ctx unsafe.Pointer) {
-	C.set_vm_ctx((*C.JavaVM)(vm), (C.jobject)(ctx))
+	C.current_vm = (*C.JavaVM)(vm)
+	C.current_ctx = (C.jobject)(ctx)
 }
 
-// TODO(hyangah): should the app package have Context? It may be useful for
-// external packages that need to access android context and vm.
-
-// Context holds global OS-specific context.
+// RunOnJVM runs fn on a new goroutine locked to an OS thread with a JNIEnv.
 //
-// Its extra methods are deliberately difficult to access because they must be
-// used with care. Their use implies the use of cgo, which probably requires
-// you understand the initialization process in the app package. Also care must
-// be taken to write both Android, iOS, and desktop-testing versions to
-// maintain portability.
-type Context struct{}
+// RunOnJVM blocks until the call to fn is complete. Any Java
+// exception or failure to attach to the JVM is returned as an error.
+//
+// The function fn takes vm, the current JavaVM*,
+// env, the current JNIEnv*, and
+// ctx, a jobject representing the global android.context.Context.
+func RunOnJVM(fn func(vm, env, ctx uintptr) error) error {
+	errch := make(chan error)
+	go func() {
+		runtime.LockOSThread()
+		defer runtime.UnlockOSThread()
 
-// AndroidContext returns a jobject for the app android.context.Context.
-func (Context) AndroidContext() unsafe.Pointer {
-	return unsafe.Pointer(C.current_ctx)
-}
+		env := C.uintptr_t(0)
+		attached := C.int(0)
+		if errStr := C.lockJNI(&env, &attached); errStr != nil {
+			errch <- errors.New(C.GoString(errStr))
+			return
+		}
+		if attached != 0 {
+			defer C.unlockJNI()
+		}
 
-// JavaVM returns a JNI *JavaVM.
-func (Context) JavaVM() unsafe.Pointer {
-	return unsafe.Pointer(C.current_vm)
+		vm := uintptr(unsafe.Pointer(C.current_vm))
+		if err := fn(vm, uintptr(env), uintptr(C.current_ctx)); err != nil {
+			errch <- err
+			return
+		}
+
+		if exc := C.checkException(env); exc != nil {
+			errch <- errors.New(C.GoString(exc))
+			C.free(unsafe.Pointer(exc))
+			return
+		}
+		errch <- nil
+	}()
+	return <-errch
 }