Commit cc8f1136b677e3b6384f00b68cd3a77a5216abc4

Sylvain Becker 2019-01-03T14:18:06

Fixed bug 4142 - Concurrency issues in Android backend Use a semaphore to prevent concurrency issues between Java Activity and Native thread code, when using 'Android_Window'. (Eg. Java sending Touch events, while native code is destroying the main SDL_Window. )

diff --git a/src/core/android/SDL_android.c b/src/core/android/SDL_android.c
index e3e5add..acfb4c4 100644
--- a/src/core/android/SDL_android.c
+++ b/src/core/android/SDL_android.c
@@ -322,6 +322,17 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativeSetupJNI)(JNIEnv *mEnv, jclass c
 {
     __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "nativeSetupJNI()");
 
+    /* Use a semaphore to prevent concurrency issues between Java Activity and Native thread code, when using 'Android_Window'.
+     * (Eg. Java sending Touch events, while native code is destroying the main SDL_Window. )
+     */
+    if (Android_ActivitySem == NULL) {
+        Android_ActivitySem = SDL_CreateSemaphore(1); /* Could this be created twice if onCreate() is called a second time ? */
+    }
+
+    if (Android_ActivitySem == NULL) {
+        __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "failed to create Android_ActivitySem semaphore");
+    }
+
     Android_JNI_SetupThread();
 
     mActivityClass = (jclass)((*mEnv)->NewGlobalRef(mEnv, cls));
@@ -558,7 +569,11 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(onNativeResize)(
                                     jint surfaceWidth, jint surfaceHeight,
                                     jint deviceWidth, jint deviceHeight, jint format, jfloat rate)
 {
+    SDL_SemWait(Android_ActivitySem);
+
     Android_SetScreenResolution(Android_Window, surfaceWidth, surfaceHeight, deviceWidth, deviceHeight, format, rate);
+
+    SDL_SemPost(Android_ActivitySem);
 }
 
 JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(onNativeOrientationChanged)(
@@ -650,6 +665,8 @@ JNIEXPORT jint JNICALL SDL_JAVA_CONTROLLER_INTERFACE(nativeRemoveHaptic)(
 /* Surface Created */
 JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(onNativeSurfaceChanged)(JNIEnv *env, jclass jcls)
 {
+    SDL_SemWait(Android_ActivitySem);
+
     if (Android_Window && Android_Window->driverdata)
     {
         SDL_VideoDevice *_this = SDL_GetVideoDevice();
@@ -666,11 +683,15 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(onNativeSurfaceChanged)(JNIEnv *env, j
 
         /* GL Context handling is done in the event loop because this function is run from the Java thread */
     }
+
+    SDL_SemPost(Android_ActivitySem);
 }
 
 /* Surface Destroyed */
 JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(onNativeSurfaceDestroyed)(JNIEnv *env, jclass jcls)
 {
+    SDL_SemWait(Android_ActivitySem);
+
     if (Android_Window && Android_Window->driverdata)
     {
         SDL_VideoDevice *_this = SDL_GetVideoDevice();
@@ -689,6 +710,8 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(onNativeSurfaceDestroyed)(JNIEnv *env,
 
         /* GL Context handling is done in the event loop because this function is run from the Java thread */
     }
+
+    SDL_SemPost(Android_ActivitySem);
 }
 
 /* Keydown */
@@ -722,7 +745,11 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(onNativeTouch)(
                                     jint touch_device_id_in, jint pointer_finger_id_in,
                                     jint action, jfloat x, jfloat y, jfloat p)
 {
+    SDL_SemWait(Android_ActivitySem);
+
     Android_OnTouch(Android_Window, touch_device_id_in, pointer_finger_id_in, action, x, y, p);
+
+    SDL_SemPost(Android_ActivitySem);
 }
 
 /* Mouse */
@@ -730,7 +757,11 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(onNativeMouse)(
                                     JNIEnv *env, jclass jcls,
                                     jint button, jint action, jfloat x, jfloat y, jboolean relative)
 {
+    SDL_SemWait(Android_ActivitySem);
+
     Android_OnMouse(Android_Window, button, action, x, y, relative);
+
+    SDL_SemPost(Android_ActivitySem);
 }
 
 /* Accelerometer */
@@ -778,6 +809,8 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativeQuit)(
 JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativePause)(
                                     JNIEnv *env, jclass cls)
 {
+    SDL_SemWait(Android_ActivitySem);
+
     __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "nativePause()");
 
     if (Android_Window) {
@@ -790,12 +823,16 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativePause)(
          * so the event loop knows to pause and (optionally) block itself */
         if (!SDL_SemValue(Android_PauseSem)) SDL_SemPost(Android_PauseSem);
     }
+
+    SDL_SemPost(Android_ActivitySem);
 }
 
 /* Resume */
 JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativeResume)(
                                     JNIEnv *env, jclass cls)
 {
+    SDL_SemWait(Android_ActivitySem);
+
     __android_log_print(ANDROID_LOG_VERBOSE, "SDL", "nativeResume()");
 
     if (Android_Window) {
@@ -816,6 +853,8 @@ JNIEXPORT void JNICALL SDL_JAVA_INTERFACE(nativeResume)(
          */
         if (!SDL_SemValue(Android_ResumeSem)) SDL_SemPost(Android_ResumeSem);
     }
+
+    SDL_SemPost(Android_ActivitySem);
 }
 
 JNIEXPORT void JNICALL SDL_JAVA_INTERFACE_INPUT_CONNECTION(nativeCommitText)(
diff --git a/src/video/android/SDL_androidevents.c b/src/video/android/SDL_androidevents.c
index 5e17504..0d3c968 100644
--- a/src/video/android/SDL_androidevents.c
+++ b/src/video/android/SDL_androidevents.c
@@ -39,8 +39,8 @@ static void ANDROIDAUDIO_ResumeDevices(void) {}
 static void ANDROIDAUDIO_PauseDevices(void) {}
 #endif
 
-static void 
-android_egl_context_restore(SDL_Window *window) 
+static void
+android_egl_context_restore(SDL_Window *window)
 {
     SDL_Event event;
     SDL_WindowData *data = (SDL_WindowData *) window->driverdata;
@@ -53,8 +53,8 @@ android_egl_context_restore(SDL_Window *window)
     }
 }
 
-static void 
-android_egl_context_backup(SDL_Window *window) 
+static void
+android_egl_context_backup(SDL_Window *window)
 {
     /* Keep a copy of the EGL Context so we can try to restore it when we resume */
     SDL_WindowData *data = (SDL_WindowData *) window->driverdata;
@@ -80,7 +80,10 @@ Android_PumpEvents(_THIS)
 #if SDL_ANDROID_BLOCK_ON_PAUSE
     if (isPaused && !isPausing) {
         /* Make sure this is the last thing we do before pausing */
+        SDL_SemWait(Android_ActivitySem);
         android_egl_context_backup(Android_Window);
+        SDL_SemPost(Android_ActivitySem);
+
         ANDROIDAUDIO_PauseDevices();
         if (SDL_SemWait(Android_ResumeSem) == 0) {
 #else
@@ -91,7 +94,9 @@ Android_PumpEvents(_THIS)
             ANDROIDAUDIO_ResumeDevices();
             /* Restore the GL Context from here, as this operation is thread dependent */
             if (!SDL_HasEvent(SDL_QUIT)) {
+                SDL_SemWait(Android_ActivitySem);
                 android_egl_context_restore(Android_Window);
+                SDL_SemPost(Android_ActivitySem);
             }
         }
     }
@@ -110,7 +115,10 @@ Android_PumpEvents(_THIS)
         }
 #else
         if (SDL_SemTryWait(Android_PauseSem) == 0) {
+            SDL_SemWait(Android_ActivitySem);
             android_egl_context_backup(Android_Window);
+            SDL_SemPost(Android_ActivitySem);
+
             ANDROIDAUDIO_PauseDevices();
             isPaused = 1;
         }
diff --git a/src/video/android/SDL_androidvideo.c b/src/video/android/SDL_androidvideo.c
index ccdb1ca..314930f 100644
--- a/src/video/android/SDL_androidvideo.c
+++ b/src/video/android/SDL_androidvideo.c
@@ -66,7 +66,7 @@ int Android_DeviceHeight = 0;
 static Uint32 Android_ScreenFormat = SDL_PIXELFORMAT_UNKNOWN;
 static int Android_ScreenRate = 0;
 
-SDL_sem *Android_PauseSem = NULL, *Android_ResumeSem = NULL;
+SDL_sem *Android_PauseSem = NULL, *Android_ResumeSem = NULL, *Android_ActivitySem = NULL;
 
 static int
 Android_Available(void)
diff --git a/src/video/android/SDL_androidvideo.h b/src/video/android/SDL_androidvideo.h
index f05906f..0ff179b 100644
--- a/src/video/android/SDL_androidvideo.h
+++ b/src/video/android/SDL_androidvideo.h
@@ -41,7 +41,7 @@ extern int Android_SurfaceWidth;
 extern int Android_SurfaceHeight;
 extern int Android_DeviceWidth;
 extern int Android_DeviceHeight;
-extern SDL_sem *Android_PauseSem, *Android_ResumeSem;
+extern SDL_sem *Android_PauseSem, *Android_ResumeSem, *Android_ActivitySem;
 
 #endif /* SDL_androidvideo_h_ */
 
diff --git a/src/video/android/SDL_androidwindow.c b/src/video/android/SDL_androidwindow.c
index 6544db5..70fe4a1 100644
--- a/src/video/android/SDL_androidwindow.c
+++ b/src/video/android/SDL_androidwindow.c
@@ -41,12 +41,14 @@ Android_CreateWindow(_THIS, SDL_Window * window)
 {
     SDL_WindowData *data;
     int retval = 0;
-    
+
+    SDL_SemWait(Android_ActivitySem);
+
     if (Android_Window) {
         retval = SDL_SetError("Android only supports one window");
         goto endfunction;
     }
-    
+
     Android_PauseSem = SDL_CreateSemaphore(0);
     Android_ResumeSem = SDL_CreateSemaphore(0);
 
@@ -67,15 +69,15 @@ Android_CreateWindow(_THIS, SDL_Window * window)
     /* One window, it always has focus */
     SDL_SetMouseFocus(window);
     SDL_SetKeyboardFocus(window);
-    
+
     data = (SDL_WindowData *) SDL_calloc(1, sizeof(*data));
     if (!data) {
         retval = SDL_OutOfMemory();
         goto endfunction;
     }
-    
+
     data->native_window = Android_JNI_GetNativeWindow();
-    
+
     if (!data->native_window) {
         SDL_free(data);
         retval = SDL_SetError("Could not fetch native window");
@@ -99,7 +101,9 @@ Android_CreateWindow(_THIS, SDL_Window * window)
     Android_Window = window;
 
 endfunction:
-    
+
+    SDL_SemPost(Android_ActivitySem);
+
     return retval;
 }
 
@@ -146,14 +150,16 @@ Android_SetWindowFullscreen(_THIS, SDL_Window *window, SDL_VideoDisplay *display
 
 void
 Android_DestroyWindow(_THIS, SDL_Window *window)
-{ 
+{
+    SDL_SemWait(Android_ActivitySem);
+
     if (window == Android_Window) {
         Android_Window = NULL;
         if (Android_PauseSem) SDL_DestroySemaphore(Android_PauseSem);
         if (Android_ResumeSem) SDL_DestroySemaphore(Android_ResumeSem);
         Android_PauseSem = NULL;
         Android_ResumeSem = NULL;
-        
+
         if (window->driverdata) {
             SDL_WindowData *data = (SDL_WindowData *) window->driverdata;
             if (data->egl_surface != EGL_NO_SURFACE) {
@@ -166,6 +172,8 @@ Android_DestroyWindow(_THIS, SDL_Window *window)
             window->driverdata = NULL;
         }
     }
+
+    SDL_SemPost(Android_ActivitySem);
 }
 
 SDL_bool