feature/android 740af4668c8: Fix input method synchronization problems

[Po Lu]

branch: feature/android
commit 740af4668c8d9bc8e4ee1e60ebeb366690fee93e
Author: Po Lu <luangruo@yahoo.com>
Commit: Po Lu <luangruo@yahoo.com>

    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.
---
 java/debug.sh                        |  13 ++---
 java/org/gnu/emacs/EmacsNative.java  |   6 ++
 java/org/gnu/emacs/EmacsService.java | 103 ++++++++++++++++++++++++++---------
 java/org/gnu/emacs/EmacsView.java    |  13 +++--
 src/android.c                        |  26 +++++++++
 5 files changed, 123 insertions(+), 38 deletions(-)

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 <https://www.gnu.org/licenses/>.
 
 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.