Bug 794981 - Part 7: Storing a void* for mThread instead of a pthread_t is both nonportable and dangerous. r=kats
authorChris Kitching <ckitching@mozilla.com>
Mon, 09 Sep 2013 08:57:37 -0400
changeset 146185 130635e9a2c1003ecc34098758cde63004bd31d1
parent 146184 6e49ee30a8e11f673f9e25972473b7575f15749d
child 146186 ea578c504f72e2de356bfaaebea991792d689443
push id25244
push userryanvm@gmail.com
push dateMon, 09 Sep 2013 20:03:14 +0000
treeherdermozilla-central@f320b8c034bd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs794981
milestone26.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 794981 - Part 7: Storing a void* for mThread instead of a pthread_t is both nonportable and dangerous. r=kats
toolkit/xre/nsAndroidStartup.cpp
widget/android/AndroidBridge.cpp
widget/android/AndroidBridge.h
--- a/toolkit/xre/nsAndroidStartup.cpp
+++ b/toolkit/xre/nsAndroidStartup.cpp
@@ -26,20 +26,20 @@
 // with local references overrunning the local refs table, among
 // other things, since GC can't ever run on them.
 
 // Note that we don't have xpcom initialized yet, so we can't use the
 // thread manager for this.  Instead, we use pthreads directly.
 
 struct AutoAttachJavaThread {
     AutoAttachJavaThread() {
-        attached = mozilla_AndroidBridge_SetMainThread((void*)pthread_self());
+        attached = mozilla_AndroidBridge_SetMainThread(pthread_self());
     }
     ~AutoAttachJavaThread() {
-        mozilla_AndroidBridge_SetMainThread(nullptr);
+        mozilla_AndroidBridge_SetMainThread(-1);
         attached = false;
     }
 
     bool attached;
 };
 
 extern "C" NS_EXPORT void
 GeckoStart(void *data, const nsXREAppData *appData)
--- a/widget/android/AndroidBridge.cpp
+++ b/widget/android/AndroidBridge.cpp
@@ -160,17 +160,17 @@ bool
 AndroidBridge::Init(JNIEnv *jEnv)
 {
     ALOG_BRIDGE("AndroidBridge::Init");
     jEnv->GetJavaVM(&mJavaVM);
 
     AutoLocalJNIFrame jniFrame(jEnv);
 
     mJNIEnv = nullptr;
-    mThread = nullptr;
+    mThread = -1;
     mGLControllerObj = nullptr;
     mOpenedGraphicsLibraries = false;
     mHasNativeBitmapAccess = false;
     mHasNativeWindowAccess = false;
     mHasNativeWindowFallback = false;
 
     initInit();
     InitStubs(jEnv);
@@ -211,27 +211,27 @@ AndroidBridge::Init(JNIEnv *jEnv)
     // jEnv should NOT be cached here by anything -- the jEnv here
     // is not valid for the real gecko main thread, which is set
     // at SetMainThread time.
 
     return true;
 }
 
 bool
-AndroidBridge::SetMainThread(void *thr)
+AndroidBridge::SetMainThread(pthread_t thr)
 {
     ALOG_BRIDGE("AndroidBridge::SetMainThread");
     if (thr) {
         mThread = thr;
         mJavaVM->GetEnv((void**) &mJNIEnv, JNI_VERSION_1_2);
         return (bool) mJNIEnv;
     }
 
     mJNIEnv = nullptr;
-    mThread = nullptr;
+    mThread = -1;
     return true;
 }
 
 // Raw JNIEnv variants.
 jstring AndroidBridge::NewJavaString(JNIEnv* env, const PRUnichar* string, uint32_t len) {
    jstring ret = env->NewString(string, len);
    if (env->ExceptionCheck()) {
        ALOG_BRIDGE("Exceptional exit of: %s", __PRETTY_FUNCTION__);
@@ -796,17 +796,17 @@ AndroidBridge::GetStaticStringField(cons
 
     result.Assign(nsJNIString(jstr, jEnv));
     jEnv->DeleteLocalRef(jstr);
     return true;
 }
 
 // Available for places elsewhere in the code to link to.
 bool
-mozilla_AndroidBridge_SetMainThread(void *thr)
+mozilla_AndroidBridge_SetMainThread(pthread_t thr)
 {
     return AndroidBridge::Bridge()->SetMainThread(thr);
 }
 
 jclass GetGeckoAppShellClass()
 {
     return mozilla::AndroidBridge::GetGeckoAppShellClass();
 }
--- a/widget/android/AndroidBridge.h
+++ b/widget/android/AndroidBridge.h
@@ -37,17 +37,17 @@
 // #define DEBUG_ANDROID_WIDGET
 
 class nsWindow;
 class nsIDOMMozSmsMessage;
 
 /* See the comment in AndroidBridge about this function before using it */
 extern "C" JNIEnv * GetJNIForThread();
 
-extern bool mozilla_AndroidBridge_SetMainThread(void *);
+extern bool mozilla_AndroidBridge_SetMainThread(pthread_t);
 extern jclass GetGeckoAppShellClass();
 
 namespace base {
 class Thread;
 } // end namespace base
 
 typedef void* EGLSurface;
 
@@ -144,17 +144,17 @@ public:
     static JavaVM *GetVM() {
         if (MOZ_LIKELY(sBridge))
             return sBridge->mJavaVM;
         return nullptr;
     }
 
     static JNIEnv *GetJNIEnv() {
         if (MOZ_LIKELY(sBridge)) {
-            if ((void*)pthread_self() != sBridge->mThread) {
+            if (!pthread_equal(pthread_self(), sBridge->mThread)) {
                 __android_log_print(ANDROID_LOG_INFO, "AndroidBridge",
                                     "###!!!!!!! Something's grabbing the JNIEnv from the wrong thread! (thr %p should be %p)",
                                     (void*)pthread_self(), (void*)sBridge->mThread);
                 MOZ_ASSERT(false, "###!!!!!!! Something's grabbing the JNIEnv from the wrong thread!");
                 return nullptr;
             }
             return sBridge->mJNIEnv;
 
@@ -166,17 +166,17 @@ public:
         return sBridge->mGeckoAppShellClass;
     }
 
     // The bridge needs to be constructed via ConstructBridge first,
     // and then once the Gecko main thread is spun up (Gecko side),
     // SetMainThread should be called which will create the JNIEnv for
     // us to use.  toolkit/xre/nsAndroidStartup.cpp calls
     // SetMainThread.
-    bool SetMainThread(void *thr);
+    bool SetMainThread(pthread_t thr);
 
     /* These are all implemented in Java */
     bool GetThreadNameJavaProfiling(uint32_t aThreadId, nsCString & aResult);
     bool GetFrameNameJavaProfiling(uint32_t aThreadId, uint32_t aSampleId, uint32_t aFrameId, nsCString & aResult);
 
     nsresult CaptureThumbnail(nsIDOMWindow *window, int32_t bufW, int32_t bufH, int32_t tabId, jobject buffer);
     void GetDisplayPort(bool aPageSizeUpdate, bool aIsBrowserContentDisplayed, int32_t tabId, nsIAndroidViewport* metrics, nsIAndroidDisplayport** displayPort);
     void ContentDocumentChanged();
@@ -336,17 +336,17 @@ protected:
     static StaticRefPtr<AndroidBridge> sBridge;
     nsTArray<nsCOMPtr<nsIMobileMessageCallback> > mSmsRequests;
 
     // the global JavaVM
     JavaVM *mJavaVM;
 
     // the JNIEnv for the main thread
     JNIEnv *mJNIEnv;
-    void *mThread;
+    pthread_t mThread;
 
     AndroidGeckoLayerClient *mLayerClient;
 
     // the android.telephony.SmsMessage class
     jclass mAndroidSmsMessageClass;
 
     AndroidBridge();
     ~AndroidBridge();