diff options
| author | Po Lu | 2023-06-04 12:04:15 +0800 |
|---|---|---|
| committer | Po Lu | 2023-06-04 12:04:15 +0800 |
| commit | 740af4668c8d9bc8e4ee1e60ebeb366690fee93e (patch) | |
| tree | 99d1b732788406053b9fbeb30609b3c4410d94d6 /java | |
| parent | c389df992a9f054f9aced4f9a267730b2221e03a (diff) | |
| download | emacs-740af4668c8d9bc8e4ee1e60ebeb366690fee93e.tar.gz emacs-740af4668c8d9bc8e4ee1e60ebeb366690fee93e.zip | |
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.
Diffstat (limited to 'java')
| -rwxr-xr-x | java/debug.sh | 13 | ||||
| -rw-r--r-- | java/org/gnu/emacs/EmacsNative.java | 6 | ||||
| -rw-r--r-- | java/org/gnu/emacs/EmacsService.java | 103 | ||||
| -rw-r--r-- | java/org/gnu/emacs/EmacsView.java | 13 |
4 files changed, 97 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 @@ | |||
| 19 | ## along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. | 19 | ## along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. |
| 20 | 20 | ||
| 21 | set -m | 21 | set -m |
| 22 | set -x | ||
| 23 | oldpwd=`pwd` | 22 | oldpwd=`pwd` |
| 24 | cd `dirname $0` | 23 | cd `dirname $0` |
| 25 | 24 | ||
| @@ -273,7 +272,7 @@ fi | |||
| 273 | gdbserver_cmd= | 272 | gdbserver_cmd= |
| 274 | is_root= | 273 | is_root= |
| 275 | if [ -z "$gdbserver" ]; then | 274 | if [ -z "$gdbserver" ]; then |
| 276 | gdbserver_bin=/system/bin/gdbserver | 275 | gdbserver_bin=/system/bin/gdbserver64 |
| 277 | else | 276 | else |
| 278 | gdbserver_bin=/data/local/tmp/gdbserver | 277 | gdbserver_bin=/data/local/tmp/gdbserver |
| 279 | gdbserver_cat="cat $gdbserver_bin | run-as $package sh -c \ | 278 | gdbserver_cat="cat $gdbserver_bin | run-as $package sh -c \ |
| @@ -312,12 +311,12 @@ rm -f /tmp/file-descriptor-stamp | |||
| 312 | if [ -z "$gdbserver" ]; then | 311 | if [ -z "$gdbserver" ]; then |
| 313 | if [ "$is_root" = "yes" ]; then | 312 | if [ "$is_root" = "yes" ]; then |
| 314 | adb -s $device shell $gdbserver_bin --multi \ | 313 | adb -s $device shell $gdbserver_bin --multi \ |
| 315 | "+/data/local/tmp/debug.$package.socket" --attach $pid >&5 & | 314 | "0.0.0.0:7564" --attach $pid >&5 & |
| 316 | gdb_socket="localfilesystem:/data/local/tmp/debug.$package.socket" | 315 | gdb_socket="tcp:7564" |
| 317 | else | 316 | else |
| 318 | adb -s $device shell run-as $package $gdbserver_bin --multi \ | 317 | adb -s $device shell $gdbserver_bin --multi \ |
| 319 | "+debug.$package.socket" --attach $pid >&5 & | 318 | "0.0.0.0:7564" --attach $pid >&5 & |
| 320 | gdb_socket="localfilesystem:$app_data_dir/debug.$package.socket" | 319 | gdb_socket="tcp:7564" |
| 321 | fi | 320 | fi |
| 322 | else | 321 | else |
| 323 | # Normally the program cannot access $gdbserver_bin when it is | 322 | # 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 | |||
| 177 | main thread's looper to respond. */ | 177 | main thread's looper to respond. */ |
| 178 | public static native void endSynchronous (); | 178 | public static native void endSynchronous (); |
| 179 | 179 | ||
| 180 | /* Prevent deadlocks while reliably allowing queries from the Emacs | ||
| 181 | thread to the main thread to complete by waiting for a query to | ||
| 182 | start from the main thread, then answer it; assume that a query | ||
| 183 | is certain to start shortly. */ | ||
| 184 | public static native void answerQuerySpin (); | ||
| 185 | |||
| 180 | /* Return whether or not KEYCODE_VOLUME_DOWN, KEYCODE_VOLUME_UP and | 186 | /* Return whether or not KEYCODE_VOLUME_DOWN, KEYCODE_VOLUME_UP and |
| 181 | KEYCODE_VOLUME_MUTE should be forwarded to Emacs. */ | 187 | KEYCODE_VOLUME_MUTE should be forwarded to Emacs. */ |
| 182 | public static native boolean shouldForwardMultimediaButtons (); | 188 | 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; | |||
| 25 | 25 | ||
| 26 | import java.util.List; | 26 | import java.util.List; |
| 27 | 27 | ||
| 28 | import java.util.concurrent.atomic.AtomicInteger; | ||
| 29 | |||
| 28 | import android.graphics.Matrix; | 30 | import android.graphics.Matrix; |
| 29 | import android.graphics.Point; | 31 | import android.graphics.Point; |
| 30 | 32 | ||
| @@ -106,8 +108,17 @@ public final class EmacsService extends Service | |||
| 106 | performing drawing calls. */ | 108 | performing drawing calls. */ |
| 107 | private static final boolean DEBUG_THREADS = false; | 109 | private static final boolean DEBUG_THREADS = false; |
| 108 | 110 | ||
| 109 | /* Whether or not onCreateInputMethod is calling getSelection. */ | 111 | /* Atomic integer used for synchronization between |
| 110 | public static volatile boolean imSyncInProgress; | 112 | icBeginSynchronous/icEndSynchronous and viewGetSelection. |
| 113 | |||
| 114 | Value is 0 if no query is in progress, 1 if viewGetSelection is | ||
| 115 | being called, and 2 if icBeginSynchronous was called. */ | ||
| 116 | public static final AtomicInteger servicingQuery; | ||
| 117 | |||
| 118 | static | ||
| 119 | { | ||
| 120 | servicingQuery = new AtomicInteger (); | ||
| 121 | }; | ||
| 111 | 122 | ||
| 112 | /* Return the directory leading to the directory in which native | 123 | /* Return the directory leading to the directory in which native |
| 113 | library files are stored on behalf of CONTEXT. */ | 124 | library files are stored on behalf of CONTEXT. */ |
| @@ -658,46 +669,79 @@ public final class EmacsService extends Service | |||
| 658 | EmacsNative.endSynchronous (); | 669 | EmacsNative.endSynchronous (); |
| 659 | } | 670 | } |
| 660 | 671 | ||
| 672 | |||
| 673 | |||
| 674 | /* IMM functions such as `updateSelection' holds an internal lock | ||
| 675 | that is also taken before `onCreateInputConnection' (in | ||
| 676 | EmacsView.java) is called; when that then asks the UI thread for | ||
| 677 | the current selection, a dead lock results. To remedy this, | ||
| 678 | reply to any synchronous queries now -- and prohibit more queries | ||
| 679 | for the duration of `updateSelection' -- if EmacsView may have | ||
| 680 | been asking for the value of the region. */ | ||
| 681 | |||
| 682 | public static void | ||
| 683 | icBeginSynchronous () | ||
| 684 | { | ||
| 685 | /* Set servicingQuery to 2, so viewGetSelection knows it shouldn't | ||
| 686 | proceed. */ | ||
| 687 | |||
| 688 | if (servicingQuery.getAndSet (2) == 1) | ||
| 689 | /* But if viewGetSelection is already in progress, answer it | ||
| 690 | first. */ | ||
| 691 | EmacsNative.answerQuerySpin (); | ||
| 692 | } | ||
| 693 | |||
| 694 | public static void | ||
| 695 | icEndSynchronous () | ||
| 696 | { | ||
| 697 | if (servicingQuery.getAndSet (0) != 2) | ||
| 698 | throw new RuntimeException ("incorrect value of `servicingQuery': " | ||
| 699 | + "likely 1"); | ||
| 700 | } | ||
| 701 | |||
| 702 | public static int[] | ||
| 703 | viewGetSelection (short window) | ||
| 704 | { | ||
| 705 | int[] selection; | ||
| 706 | |||
| 707 | /* See if a query is already in progress from the other | ||
| 708 | direction. */ | ||
| 709 | if (!servicingQuery.compareAndSet (0, 1)) | ||
| 710 | return null; | ||
| 711 | |||
| 712 | /* Now call the regular getSelection. Note that this can't race | ||
| 713 | with answerQuerySpin, as `android_servicing_query' can never be | ||
| 714 | 2 when icBeginSynchronous is called, so a query will always be | ||
| 715 | started. */ | ||
| 716 | selection = EmacsNative.getSelection (window); | ||
| 717 | |||
| 718 | /* Finally, clear servicingQuery if its value is still 1. If a | ||
| 719 | query has started from the other side, it ought to be 2. */ | ||
| 720 | |||
| 721 | servicingQuery.compareAndSet (1, 0); | ||
| 722 | return selection; | ||
| 723 | } | ||
| 724 | |||
| 725 | |||
| 726 | |||
| 661 | public void | 727 | public void |
| 662 | updateIC (EmacsWindow window, int newSelectionStart, | 728 | updateIC (EmacsWindow window, int newSelectionStart, |
| 663 | int newSelectionEnd, int composingRegionStart, | 729 | int newSelectionEnd, int composingRegionStart, |
| 664 | int composingRegionEnd) | 730 | int composingRegionEnd) |
| 665 | { | 731 | { |
| 666 | boolean wasSynchronous; | ||
| 667 | |||
| 668 | if (DEBUG_IC) | 732 | if (DEBUG_IC) |
| 669 | Log.d (TAG, ("updateIC: " + window + " " + newSelectionStart | 733 | Log.d (TAG, ("updateIC: " + window + " " + newSelectionStart |
| 670 | + " " + newSelectionEnd + " " | 734 | + " " + newSelectionEnd + " " |
| 671 | + composingRegionStart + " " | 735 | + composingRegionStart + " " |
| 672 | + composingRegionEnd)); | 736 | + composingRegionEnd)); |
| 673 | 737 | ||
| 674 | /* `updateSelection' holds an internal lock that is also taken | 738 | icBeginSynchronous (); |
| 675 | before `onCreateInputConnection' (in EmacsView.java) is called; | ||
| 676 | when that then asks the UI thread for the current selection, a | ||
| 677 | dead lock results. To remedy this, reply to any synchronous | ||
| 678 | queries now -- and prohibit more queries for the duration of | ||
| 679 | `updateSelection' -- if EmacsView may have been asking for the | ||
| 680 | value of the region. */ | ||
| 681 | |||
| 682 | wasSynchronous = false; | ||
| 683 | if (EmacsService.imSyncInProgress) | ||
| 684 | { | ||
| 685 | /* `beginSynchronous' will answer any outstanding queries and | ||
| 686 | signal that one is now in progress, thereby preventing | ||
| 687 | `getSelection' from blocking. */ | ||
| 688 | |||
| 689 | EmacsNative.beginSynchronous (); | ||
| 690 | wasSynchronous = true; | ||
| 691 | } | ||
| 692 | |||
| 693 | window.view.imManager.updateSelection (window.view, | 739 | window.view.imManager.updateSelection (window.view, |
| 694 | newSelectionStart, | 740 | newSelectionStart, |
| 695 | newSelectionEnd, | 741 | newSelectionEnd, |
| 696 | composingRegionStart, | 742 | composingRegionStart, |
| 697 | composingRegionEnd); | 743 | composingRegionEnd); |
| 698 | 744 | icEndSynchronous (); | |
| 699 | if (wasSynchronous) | ||
| 700 | EmacsNative.endSynchronous (); | ||
| 701 | } | 745 | } |
| 702 | 746 | ||
| 703 | public void | 747 | public void |
| @@ -707,7 +751,10 @@ public final class EmacsService extends Service | |||
| 707 | Log.d (TAG, "resetIC: " + window); | 751 | Log.d (TAG, "resetIC: " + window); |
| 708 | 752 | ||
| 709 | window.view.setICMode (icMode); | 753 | window.view.setICMode (icMode); |
| 754 | |||
| 755 | icBeginSynchronous (); | ||
| 710 | window.view.imManager.restartInput (window.view); | 756 | window.view.imManager.restartInput (window.view); |
| 757 | icEndSynchronous (); | ||
| 711 | } | 758 | } |
| 712 | 759 | ||
| 713 | public void | 760 | public void |
| @@ -733,11 +780,15 @@ public final class EmacsService extends Service | |||
| 733 | 0); | 780 | 0); |
| 734 | info = builder.build (); | 781 | info = builder.build (); |
| 735 | 782 | ||
| 783 | |||
| 784 | |||
| 736 | if (DEBUG_IC) | 785 | if (DEBUG_IC) |
| 737 | Log.d (TAG, ("updateCursorAnchorInfo: " + x + " " + y | 786 | Log.d (TAG, ("updateCursorAnchorInfo: " + x + " " + y |
| 738 | + " " + yBaseline + "-" + yBottom)); | 787 | + " " + yBaseline + "-" + yBottom)); |
| 739 | 788 | ||
| 789 | icBeginSynchronous (); | ||
| 740 | window.view.imManager.updateCursorAnchorInfo (window.view, info); | 790 | window.view.imManager.updateCursorAnchorInfo (window.view, info); |
| 791 | icEndSynchronous (); | ||
| 741 | } | 792 | } |
| 742 | 793 | ||
| 743 | /* Open a content URI described by the bytes BYTES, a non-terminated | 794 | /* 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 | |||
| 630 | /* Obtain the current position of point and set it as the | 630 | /* Obtain the current position of point and set it as the |
| 631 | selection. Don't do this under one specific situation: if | 631 | selection. Don't do this under one specific situation: if |
| 632 | `android_update_ic' is being called in the main thread, trying | 632 | `android_update_ic' is being called in the main thread, trying |
| 633 | to synchronize with it can cause a dead lock in the IM | 633 | to synchronize with it can cause a dead lock in the IM manager. |
| 634 | manager. */ | 634 | See icBeginSynchronous in EmacsService.java for more |
| 635 | details. */ | ||
| 635 | 636 | ||
| 636 | EmacsService.imSyncInProgress = true; | 637 | selection = EmacsService.viewGetSelection (window.handle); |
| 637 | selection = EmacsNative.getSelection (window.handle); | ||
| 638 | EmacsService.imSyncInProgress = false; | ||
| 639 | 638 | ||
| 640 | if (selection != null) | 639 | if (selection != null) |
| 641 | Log.d (TAG, "onCreateInputConnection: current selection is: " | 640 | Log.d (TAG, "onCreateInputConnection: current selection is: " |
| @@ -664,6 +663,10 @@ public final class EmacsView extends ViewGroup | |||
| 664 | 663 | ||
| 665 | if (inputConnection == null) | 664 | if (inputConnection == null) |
| 666 | inputConnection = new EmacsInputConnection (this); | 665 | inputConnection = new EmacsInputConnection (this); |
| 666 | else | ||
| 667 | /* Reset the composing region, in case there is still composing | ||
| 668 | text. */ | ||
| 669 | inputConnection.finishComposingText (); | ||
| 667 | 670 | ||
| 668 | /* Return the input connection. */ | 671 | /* Return the input connection. */ |
| 669 | return inputConnection; | 672 | return inputConnection; |