diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc index 2185aef20..6910aa4a3 100644 --- a/talk/app/webrtc/java/jni/peerconnection_jni.cc +++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc @@ -152,7 +152,10 @@ namespace { static JavaVM* g_jvm = NULL; // Set in JNI_OnLoad(). static pthread_once_t g_jni_ptr_once = PTHREAD_ONCE_INIT; -static pthread_key_t g_jni_ptr; // Key for per-thread JNIEnv* data. +// Key for per-thread JNIEnv* data. Non-NULL in threads attached to |g_jvm| by +// AttachCurrentThreadIfNeeded(), NULL in unattached threads and threads that +// were attached by the JVM because of a Java->native call. +static pthread_key_t g_jni_ptr; // Return thread ID as a string. static std::string GetThreadId() { @@ -170,9 +173,32 @@ static std::string GetThreadName() { return std::string(name); } -static void ThreadDestructor(void* unused) { +// Return a |JNIEnv*| usable on this thread or NULL if this thread is detached. +static JNIEnv* GetEnv() { + void* env = NULL; + jint status = g_jvm->GetEnv(&env, JNI_VERSION_1_6); + CHECK(((env != NULL) && (status == JNI_OK)) || + ((env == NULL) && (status == JNI_EDETACHED)), + "Unexpected GetEnv return: " << status << ":" << env); + return reinterpret_cast(env); +} + +static void ThreadDestructor(void* prev_jni_ptr) { + // This function only runs on threads where |g_jni_ptr| is non-NULL, meaning + // we were responsible for originally attaching the thread, so are responsible + // for detaching it now. However, because some JVM implementations (notably + // Oracle's http://goo.gl/eHApYT) also use the pthread_key_create mechanism, + // the JVMs accounting info for this thread may already be wiped out by the + // time this is called. Thus it may appear we are already detached even though + // it was our responsibility to detach! Oh well. + if (!GetEnv()) + return; + + CHECK(GetEnv() == prev_jni_ptr, + "Detaching from another thread: " << prev_jni_ptr << ":" << GetEnv()); jint status = g_jvm->DetachCurrentThread(); CHECK(status == JNI_OK, "Failed to detach thread: " << status); + CHECK(!GetEnv(), "Detaching was a successful no-op???"); } static void CreateJNIPtrKey() { @@ -180,28 +206,29 @@ static void CreateJNIPtrKey() { "pthread_key_create"); } -// Deal with difference in signatures between Oracle's jni.h and Android's. +// Return a |JNIEnv*| usable on this thread. Attaches to |g_jvm| if necessary. static JNIEnv* AttachCurrentThreadIfNeeded() { - CHECK(!pthread_once(&g_jni_ptr_once, &CreateJNIPtrKey), - "pthread_once"); - JNIEnv* jni = reinterpret_cast(pthread_getspecific(g_jni_ptr)); - if (jni == NULL) { + JNIEnv* jni = GetEnv(); + if (jni) + return jni; + CHECK(!pthread_getspecific(g_jni_ptr), "TLS has a JNIEnv* but not attached?"); + + char* name = strdup((GetThreadName() + " - " + GetThreadId()).c_str()); + JavaVMAttachArgs args; + args.version = JNI_VERSION_1_6; + args.name = name; + args.group = NULL; + // Deal with difference in signatures between Oracle's jni.h and Android's. #ifdef _JAVASOFT_JNI_H_ // Oracle's jni.h violates the JNI spec! - void* env; + void* env = NULL; #else - JNIEnv* env; + JNIEnv* env = NULL; #endif - char* name = strdup((GetThreadName() + " - " + GetThreadId()).c_str()); - JavaVMAttachArgs args; - args.version = JNI_VERSION_1_6; - args.name = name; - args.group = NULL; - CHECK(!g_jvm->AttachCurrentThread(&env, &args), "Failed to attach thread"); - free(name); - CHECK(env, "AttachCurrentThread handed back NULL!"); - jni = reinterpret_cast(env); - CHECK(!pthread_setspecific(g_jni_ptr, jni), "pthread_setspecific"); - } + CHECK(!g_jvm->AttachCurrentThread(&env, &args), "Failed to attach thread"); + free(name); + CHECK(env, "AttachCurrentThread handed back NULL!"); + jni = reinterpret_cast(env); + CHECK(!pthread_setspecific(g_jni_ptr, jni), "pthread_setspecific"); return jni; } @@ -1710,6 +1737,8 @@ extern "C" jint JNIEXPORT JNICALL JNI_OnLoad(JavaVM *jvm, void *reserved) { g_jvm = jvm; CHECK(g_jvm, "JNI_OnLoad handed NULL?"); + CHECK(!pthread_once(&g_jni_ptr_once, &CreateJNIPtrKey), "pthread_once"); + CHECK(talk_base::InitializeSSL(), "Failed to InitializeSSL()"); JNIEnv* jni; @@ -1725,6 +1754,7 @@ extern "C" void JNIEXPORT JNICALL JNI_OnUnLoad(JavaVM *jvm, void *reserved) { delete g_class_reference_holder; g_class_reference_holder = NULL; CHECK(talk_base::CleanupSSL(), "Failed to CleanupSSL()"); + g_jvm = NULL; } static DataChannelInterface* ExtractNativeDC(JNIEnv* jni, jobject j_dc) { diff --git a/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java b/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java index 73a7b9e0f..b4c96b170 100644 --- a/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java +++ b/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java @@ -505,6 +505,28 @@ public class PeerConnectionTest extends TestCase { @Test public void testCompleteSession() throws Exception { + doTest(); + } + + @Test + public void testCompleteSessionOnNonMainThread() throws Exception { + final Exception[] exceptionHolder = new Exception[1]; + Thread nonMainThread = new Thread("PeerConnectionTest-nonMainThread") { + @Override public void run() { + try { + doTest(); + } catch (Exception e) { + exceptionHolder[0] = e; + } + } + }; + nonMainThread.start(); + nonMainThread.join(); + if (exceptionHolder[0] != null) + throw exceptionHolder[0]; + } + + private void doTest() throws Exception { CountDownLatch testDone = new CountDownLatch(1); System.gc(); // Encourage any GC-related threads to start up. //TreeSet threadsBeforeTest = allThreads(); diff --git a/talk/media/devices/linuxdevicemanager.cc b/talk/media/devices/linuxdevicemanager.cc index e3e55ff79..8e58d99da 100644 --- a/talk/media/devices/linuxdevicemanager.cc +++ b/talk/media/devices/linuxdevicemanager.cc @@ -302,6 +302,12 @@ LinuxDeviceWatcher::LinuxDeviceWatcher(DeviceManagerInterface* dm) LinuxDeviceWatcher::~LinuxDeviceWatcher() { } +static talk_base::PhysicalSocketServer* CurrentSocketServer() { + talk_base::SocketServer* ss = + talk_base::ThreadManager::Instance()->WrapCurrentThread()->socketserver(); + return reinterpret_cast(ss); +} + bool LinuxDeviceWatcher::Start() { // We deliberately return true in the failure paths here because libudev is // not a critical component of a Linux system so it may not be present/usable, @@ -341,16 +347,14 @@ bool LinuxDeviceWatcher::Start() { LOG_ERR(LS_ERROR) << "udev_monitor_enable_receiving()"; return true; } - static_cast( - talk_base::Thread::Current()->socketserver())->Add(this); + CurrentSocketServer()->Add(this); registered_ = true; return true; } void LinuxDeviceWatcher::Stop() { if (registered_) { - static_cast( - talk_base::Thread::Current()->socketserver())->Remove(this); + CurrentSocketServer()->Remove(this); registered_ = false; } if (udev_monitor_) { @@ -380,8 +384,7 @@ void LinuxDeviceWatcher::OnEvent(uint32 ff, int err) { LOG_ERR(LS_WARNING) << "udev_monitor_receive_device()"; // Stop listening to avoid potential livelock (an fd with EOF in it is // always considered readable). - static_cast( - talk_base::Thread::Current()->socketserver())->Remove(this); + CurrentSocketServer()->Remove(this); registered_ = false; return; }