Skip to content

Commit

Permalink
Fixed bug 4142 - Concurrency issues in Android backend
Browse files Browse the repository at this point in the history 
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. )
  • Loading branch information
@1bsyl
1bsyl committed Jan 3, 2019
1 parent d11f761 commit cc8f113
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 14 deletions.
39 changes: 39 additions & 0 deletions src/core/android/SDL_android.c
View file
Expand Up @@ -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));
Expand Down Expand Up @@ -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)(
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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 */
Expand Down Expand Up @@ -722,15 +745,23 @@ 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 */
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 */
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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)(
Expand Down
16 changes: 12 additions & 4 deletions src/video/android/SDL_androidevents.c
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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);
}
}
}
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/video/android/SDL_androidvideo.c
View file
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/video/android/SDL_androidvideo.h
View file
Expand Up @@ -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_ */

Expand Down
24 changes: 16 additions & 8 deletions src/video/android/SDL_androidwindow.c
View file
Expand Up @@ -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);

Expand All @@ -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");
Expand All @@ -99,7 +101,9 @@ Android_CreateWindow(_THIS, SDL_Window * window)
Android_Window = window;

endfunction:


SDL_SemPost(Android_ActivitySem);

return retval;
}

Expand Down Expand Up @@ -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) {
Expand All @@ -166,6 +172,8 @@ Android_DestroyWindow(_THIS, SDL_Window *window)
window->driverdata = NULL;
}
}

SDL_SemPost(Android_ActivitySem);
}

SDL_bool
Expand Down

0 comments on commit cc8f113

Please sign in to comment.