From: Po Lu Date: Sun, 4 Jun 2023 04:04:15 +0000 (+0800) Subject: Fix input method synchronization problems X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=740af4668c8d9bc8e4ee1e60ebeb366690fee93e;p=emacs.git Fix input method synchronization problems * java/debug.sh (gdbserver_cmd, is_root): Prefer TCP again. * java/org/gnu/emacs/EmacsNative.java (EmacsNative): New function `queryAndSpin'. * java/org/gnu/emacs/EmacsService.java (EmacsService) (icBeginSynchronous, icEndSynchronous, viewGetSelection): New synchronization functions. (resetIC, updateCursorAnchorInfo): Call those instead. * java/org/gnu/emacs/EmacsView.java (onCreateInputConnection): Call viewGetSelection. * src/android.c (JNICALL, android_answer_query_spin): New functions. --- diff --git a/java/debug.sh b/java/debug.sh index 0458003fe72..d6e439bec90 100755 --- a/java/debug.sh +++ b/java/debug.sh @@ -19,7 +19,6 @@ ## along with GNU Emacs. If not, see . set -m -set -x oldpwd=`pwd` cd `dirname $0` @@ -273,7 +272,7 @@ fi gdbserver_cmd= is_root= if [ -z "$gdbserver" ]; then - gdbserver_bin=/system/bin/gdbserver + gdbserver_bin=/system/bin/gdbserver64 else gdbserver_bin=/data/local/tmp/gdbserver gdbserver_cat="cat $gdbserver_bin | run-as $package sh -c \ @@ -312,12 +311,12 @@ rm -f /tmp/file-descriptor-stamp if [ -z "$gdbserver" ]; then if [ "$is_root" = "yes" ]; then adb -s $device shell $gdbserver_bin --multi \ - "+/data/local/tmp/debug.$package.socket" --attach $pid >&5 & - gdb_socket="localfilesystem:/data/local/tmp/debug.$package.socket" + "0.0.0.0:7564" --attach $pid >&5 & + gdb_socket="tcp:7564" else - adb -s $device shell run-as $package $gdbserver_bin --multi \ - "+debug.$package.socket" --attach $pid >&5 & - gdb_socket="localfilesystem:$app_data_dir/debug.$package.socket" + adb -s $device shell $gdbserver_bin --multi \ + "0.0.0.0:7564" --attach $pid >&5 & + gdb_socket="tcp:7564" fi else # Normally the program cannot access $gdbserver_bin when it is diff --git a/java/org/gnu/emacs/EmacsNative.java b/java/org/gnu/emacs/EmacsNative.java index eb75201088b..f03736fe614 100644 --- a/java/org/gnu/emacs/EmacsNative.java +++ b/java/org/gnu/emacs/EmacsNative.java @@ -177,6 +177,12 @@ public final class EmacsNative main thread's looper to respond. */ public static native void endSynchronous (); + /* Prevent deadlocks while reliably allowing queries from the Emacs + thread to the main thread to complete by waiting for a query to + start from the main thread, then answer it; assume that a query + is certain to start shortly. */ + public static native void answerQuerySpin (); + /* Return whether or not KEYCODE_VOLUME_DOWN, KEYCODE_VOLUME_UP and KEYCODE_VOLUME_MUTE should be forwarded to Emacs. */ public static native boolean shouldForwardMultimediaButtons (); diff --git a/java/org/gnu/emacs/EmacsService.java b/java/org/gnu/emacs/EmacsService.java index dde60e1c5af..6d70536faf0 100644 --- a/java/org/gnu/emacs/EmacsService.java +++ b/java/org/gnu/emacs/EmacsService.java @@ -25,6 +25,8 @@ import java.io.UnsupportedEncodingException; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + import android.graphics.Matrix; import android.graphics.Point; @@ -106,8 +108,17 @@ public final class EmacsService extends Service performing drawing calls. */ private static final boolean DEBUG_THREADS = false; - /* Whether or not onCreateInputMethod is calling getSelection. */ - public static volatile boolean imSyncInProgress; + /* Atomic integer used for synchronization between + icBeginSynchronous/icEndSynchronous and viewGetSelection. + + Value is 0 if no query is in progress, 1 if viewGetSelection is + being called, and 2 if icBeginSynchronous was called. */ + public static final AtomicInteger servicingQuery; + + static + { + servicingQuery = new AtomicInteger (); + }; /* Return the directory leading to the directory in which native library files are stored on behalf of CONTEXT. */ @@ -658,46 +669,79 @@ public final class EmacsService extends Service EmacsNative.endSynchronous (); } + + + /* IMM functions such as `updateSelection' holds an internal lock + that is also taken before `onCreateInputConnection' (in + EmacsView.java) is called; when that then asks the UI thread for + the current selection, a dead lock results. To remedy this, + reply to any synchronous queries now -- and prohibit more queries + for the duration of `updateSelection' -- if EmacsView may have + been asking for the value of the region. */ + + public static void + icBeginSynchronous () + { + /* Set servicingQuery to 2, so viewGetSelection knows it shouldn't + proceed. */ + + if (servicingQuery.getAndSet (2) == 1) + /* But if viewGetSelection is already in progress, answer it + first. */ + EmacsNative.answerQuerySpin (); + } + + public static void + icEndSynchronous () + { + if (servicingQuery.getAndSet (0) != 2) + throw new RuntimeException ("incorrect value of `servicingQuery': " + + "likely 1"); + } + + public static int[] + viewGetSelection (short window) + { + int[] selection; + + /* See if a query is already in progress from the other + direction. */ + if (!servicingQuery.compareAndSet (0, 1)) + return null; + + /* Now call the regular getSelection. Note that this can't race + with answerQuerySpin, as `android_servicing_query' can never be + 2 when icBeginSynchronous is called, so a query will always be + started. */ + selection = EmacsNative.getSelection (window); + + /* Finally, clear servicingQuery if its value is still 1. If a + query has started from the other side, it ought to be 2. */ + + servicingQuery.compareAndSet (1, 0); + return selection; + } + + + public void updateIC (EmacsWindow window, int newSelectionStart, int newSelectionEnd, int composingRegionStart, int composingRegionEnd) { - boolean wasSynchronous; - if (DEBUG_IC) Log.d (TAG, ("updateIC: " + window + " " + newSelectionStart + " " + newSelectionEnd + " " + composingRegionStart + " " + composingRegionEnd)); - /* `updateSelection' holds an internal lock that is also taken - before `onCreateInputConnection' (in EmacsView.java) is called; - when that then asks the UI thread for the current selection, a - dead lock results. To remedy this, reply to any synchronous - queries now -- and prohibit more queries for the duration of - `updateSelection' -- if EmacsView may have been asking for the - value of the region. */ - - wasSynchronous = false; - if (EmacsService.imSyncInProgress) - { - /* `beginSynchronous' will answer any outstanding queries and - signal that one is now in progress, thereby preventing - `getSelection' from blocking. */ - - EmacsNative.beginSynchronous (); - wasSynchronous = true; - } - + icBeginSynchronous (); window.view.imManager.updateSelection (window.view, newSelectionStart, newSelectionEnd, composingRegionStart, composingRegionEnd); - - if (wasSynchronous) - EmacsNative.endSynchronous (); + icEndSynchronous (); } public void @@ -707,7 +751,10 @@ public final class EmacsService extends Service Log.d (TAG, "resetIC: " + window); window.view.setICMode (icMode); + + icBeginSynchronous (); window.view.imManager.restartInput (window.view); + icEndSynchronous (); } public void @@ -733,11 +780,15 @@ public final class EmacsService extends Service 0); info = builder.build (); + + if (DEBUG_IC) Log.d (TAG, ("updateCursorAnchorInfo: " + x + " " + y + " " + yBaseline + "-" + yBottom)); + icBeginSynchronous (); window.view.imManager.updateCursorAnchorInfo (window.view, info); + icEndSynchronous (); } /* Open a content URI described by the bytes BYTES, a non-terminated diff --git a/java/org/gnu/emacs/EmacsView.java b/java/org/gnu/emacs/EmacsView.java index bb450bb8e6b..c223dfa7911 100644 --- a/java/org/gnu/emacs/EmacsView.java +++ b/java/org/gnu/emacs/EmacsView.java @@ -630,12 +630,11 @@ public final class EmacsView extends ViewGroup /* Obtain the current position of point and set it as the selection. Don't do this under one specific situation: if `android_update_ic' is being called in the main thread, trying - to synchronize with it can cause a dead lock in the IM - manager. */ + to synchronize with it can cause a dead lock in the IM manager. + See icBeginSynchronous in EmacsService.java for more + details. */ - EmacsService.imSyncInProgress = true; - selection = EmacsNative.getSelection (window.handle); - EmacsService.imSyncInProgress = false; + selection = EmacsService.viewGetSelection (window.handle); if (selection != null) Log.d (TAG, "onCreateInputConnection: current selection is: " @@ -664,6 +663,10 @@ public final class EmacsView extends ViewGroup if (inputConnection == null) inputConnection = new EmacsInputConnection (this); + else + /* Reset the composing region, in case there is still composing + text. */ + inputConnection.finishComposingText (); /* Return the input connection. */ return inputConnection; diff --git a/src/android.c b/src/android.c index e74d40a0cdb..f59c0d9e5d2 100644 --- a/src/android.c +++ b/src/android.c @@ -2997,6 +2997,7 @@ NATIVE_NAME (blitRect) (JNIEnv *env, jobject object, static void android_begin_query (void); static void android_end_query (void); +static void android_answer_query_spin (void); JNIEXPORT void JNICALL NATIVE_NAME (beginSynchronous) (JNIEnv *env, jobject object) @@ -3014,6 +3015,14 @@ NATIVE_NAME (endSynchronous) (JNIEnv *env, jobject object) android_end_query (); } +JNIEXPORT void JNICALL +NATIVE_NAME (answerQuerySpin) (JNIEnv *env, jobject object) +{ + JNI_STACK_ALIGNMENT_PROLOGUE; + + android_answer_query_spin (); +} + #ifdef __clang__ #pragma clang diagnostic pop #else @@ -7041,6 +7050,23 @@ android_answer_query (void) sem_post (&android_query_sem); } +/* Like `android_answer_query'. However, the query may not have + begun; spin until it has. */ + +static void +android_answer_query_spin (void) +{ + int n; + + while (!(n = __atomic_load_n (&android_servicing_query, + __ATOMIC_SEQ_CST))) + eassert (!n); + + /* Note that this function is supposed to be called before + `android_begin_query' starts, so clear the service flag. */ + android_check_query (); +} + /* Notice that the Emacs thread will start blocking waiting for a response from the UI thread. Process any pending queries from the UI thread.