From cc8f1136b677e3b6384f00b68cd3a77a5216abc4 Mon Sep 17 00:00:00 2001 From: Sylvain Becker Date: Thu, 3 Jan 2019 14:18:06 +0100 Subject: [PATCH] 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. ) --- src/core/android/SDL_android.c | 39 +++++++++++++++++++++++++++ src/video/android/SDL_androidevents.c | 16 ++++++++--- src/video/android/SDL_androidvideo.c | 2 +- src/video/android/SDL_androidvideo.h | 2 +- src/video/android/SDL_androidwindow.c | 24 +++++++++++------ 5 files changed, 69 insertions(+), 14 deletions(-) diff --git a/src/core/android/SDL_android.c b/src/core/android/SDL_android.c index e3e5addeb..acfb4c4d9 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 5e1750421..0d3c96841 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 ccdb1ca3c..314930fe8 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 f05906fc2..0ff179b65 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 6544db55e..70fe4a183 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