android: make replacement-method dispatch lock-free (arm64 deadlock fix)

lib/android.js

@@ -1802,123 +1802,153 @@ function makeArtController (api, vm) {
   const code = `
 #include <gum/guminterceptor.h>
 
-extern GMutex lock;
-extern GHashTable * methods;
-extern GHashTable * replacements;
 extern gpointer last_seen_art_method;
+extern gpointer entries;
 
 extern gpointer get_oat_quick_method_header_impl (gpointer method, gpointer pc);
 
+/*
+ * Hooked-method mappings are published as an immutable singly-linked
+ * list. Hook install/revert always runs on the JS thread (single
+ * writer), which prepends a fresh entry and republishes the head with a
+ * plain aligned pointer store; the trampolines on arbitrary ART threads
+ * walk the list lock-free, so they never block against ART thread
+ * suspension. Retired lists are deliberately leaked: there is no safe
+ * reclamation point without suspending all ART threads.
+ */
+typedef struct _ReplacementEntry ReplacementEntry;
+
+struct _ReplacementEntry
+{
+  gpointer original;
+  gpointer replacement;
+  ReplacementEntry * next;
+};
+
+static ReplacementEntry *
+replacement_entry_new (gpointer original, gpointer replacement)
+{
+  ReplacementEntry * entry;
+
+  entry = g_new (ReplacementEntry, 1);
+  entry->original = original;
+  entry->replacement = replacement;
+  entry->next = NULL;
+
+  return entry;
+}
+
 void
 init (void)
 {
-  g_mutex_init (&lock);
-  methods = g_hash_table_new_full (NULL, NULL, NULL, NULL);
-  replacements = g_hash_table_new_full (NULL, NULL, NULL, NULL);
+  entries = NULL;
 }
 
 void
 finalize (void)
 {
-  g_hash_table_unref (replacements);
-  g_hash_table_unref (methods);
-  g_mutex_clear (&lock);
+  ReplacementEntry * entry, * next;
+
+  for (entry = entries; entry != NULL; entry = next)
+  {
+    next = entry->next;
+    g_free (entry);
+  }
 }
 
 gboolean
 is_replacement_method (gpointer method)
 {
-  gboolean is_replacement;
-
-  g_mutex_lock (&lock);
+  ReplacementEntry * entry;
 
-  is_replacement = g_hash_table_contains (replacements, method);
-
-  g_mutex_unlock (&lock);
+  for (entry = entries; entry != NULL; entry = entry->next)
+  {
+    if (entry->replacement == method)
+      return TRUE;
+  }
 
-  return is_replacement;
+  return FALSE;
 }
 
 gpointer
 get_replacement_method (gpointer original_method)
 {
-  gpointer replacement_method;
-
-  g_mutex_lock (&lock);
+  ReplacementEntry * entry;
 
-  replacement_method = g_hash_table_lookup (methods, original_method);
-
-  g_mutex_unlock (&lock);
+  for (entry = entries; entry != NULL; entry = entry->next)
+  {
+    if (entry->original == original_method)
+      return entry->replacement;
+  }
 
-  return replacement_method;
+  return NULL;
 }
 
 void
 set_replacement_method (gpointer original_method,
                         gpointer replacement_method)
 {
-  g_mutex_lock (&lock);
+  ReplacementEntry * entry;
 
-  g_hash_table_insert (methods, original_method, replacement_method);
-  g_hash_table_insert (replacements, replacement_method, original_method);
+  entry = replacement_entry_new (original_method, replacement_method);
+  entry->next = entries;
 
-  g_mutex_unlock (&lock);
+  entries = entry;
 }
 
 void
 synchronize_replacement_methods (guint quick_code_offset,
                                  void * nterp_entrypoint,
                                  void * quick_to_interpreter_bridge)
 {
-  GHashTableIter iter;
-  gpointer hooked_method, replacement_method;
-
-  g_mutex_lock (&lock);
+  ReplacementEntry * entry;
 
-  g_hash_table_iter_init (&iter, methods);
-  while (g_hash_table_iter_next (&iter, &hooked_method, &replacement_method))
+  for (entry = entries; entry != NULL; entry = entry->next)
   {
     void ** quick_code;
 
-    *((uint32_t *) replacement_method) = *((uint32_t *) hooked_method);
+    *((uint32_t *) entry->replacement) = *((uint32_t *) entry->original);
 
-    quick_code = hooked_method + quick_code_offset;
+    quick_code = entry->original + quick_code_offset;
     if (*quick_code == nterp_entrypoint)
       *quick_code = quick_to_interpreter_bridge;
   }
-
-  g_mutex_unlock (&lock);
 }
 
 void
 delete_replacement_method (gpointer original_method)
 {
-  gpointer replacement_method;
+  ReplacementEntry * old_entry, * entry;
 
-  g_mutex_lock (&lock);
+  old_entry = entries;
+  entries = NULL;
 
-  replacement_method = g_hash_table_lookup (methods, original_method);
-  if (replacement_method != NULL)
+  while (old_entry != NULL)
   {
-    g_hash_table_remove (methods, original_method);
-    g_hash_table_remove (replacements, replacement_method);
-  }
+    if (old_entry->original != original_method)
+    {
+      entry = replacement_entry_new (old_entry->original,
+                                     old_entry->replacement);
+      entry->next = entries;
+      entries = entry;
+    }
 
-  g_mutex_unlock (&lock);
+    old_entry = old_entry->next;
+  }
 }
 
 gpointer
 translate_method (gpointer method)
 {
-  gpointer translated_method;
-
-  g_mutex_lock (&lock);
+  ReplacementEntry * entry;
 
-  translated_method = g_hash_table_lookup (replacements, method);
-
-  g_mutex_unlock (&lock);
+  for (entry = entries; entry != NULL; entry = entry->next)
+  {
+    if (entry->replacement == method)
+      return entry->original;
+  }
 
-  return (translated_method != NULL) ? translated_method : method;
+  return method;
 }
 
 gpointer
@@ -2003,39 +2033,24 @@ on_art_method_pretty_method (GumInvocationContext * ic)
 void
 on_leave_gc_concurrent_copying_copying_phase (GumInvocationContext * ic)
 {
-  GHashTableIter iter;
-  gpointer hooked_method, replacement_method;
-
-  g_mutex_lock (&lock);
+  ReplacementEntry * entry;
 
-  g_hash_table_iter_init (&iter, methods);
-  while (g_hash_table_iter_next (&iter, &hooked_method, &replacement_method))
-    *((uint32_t *) replacement_method) = *((uint32_t *) hooked_method);
-
-  g_mutex_unlock (&lock);
+  for (entry = entries; entry != NULL; entry = entry->next)
+    *((uint32_t *) entry->replacement) = *((uint32_t *) entry->original);
 }
 `;
 
-  const lockSize = 8;
-  const methodsSize = pointerSize;
-  const replacementsSize = pointerSize;
-  const lastSeenArtMethodSize = pointerSize;
-
-  const data = Memory.alloc(lockSize + methodsSize + replacementsSize + lastSeenArtMethodSize);
+  const data = Memory.alloc(pointerSize * 2);
 
-  const lock = data;
-  const methods = lock.add(lockSize);
-  const replacements = methods.add(methodsSize);
-  const lastSeenArtMethod = replacements.add(replacementsSize);
+  const entries = data;
+  const lastSeenArtMethod = entries.add(pointerSize);
 
   const getOatQuickMethodHeaderImpl = api.find((pointerSize === 4)
     ? '_ZN3art9ArtMethod23GetOatQuickMethodHeaderEj'
     : '_ZN3art9ArtMethod23GetOatQuickMethodHeaderEm');
 
   const cm = new CModule(code, {
-    lock,
-    methods,
-    replacements,
+    entries,
     last_seen_art_method: lastSeenArtMethod,
     get_oat_quick_method_header_impl: getOatQuickMethodHeaderImpl ?? ptr('0xdeadbeef')
   });

repro/README.md

@@ -0,0 +1,69 @@
+# arm64 dispatch deadlock — reproduction
+
+Reproduces the process-wide deadlock described in the PR: with
+`.implementation` hooks installed, frida-java-bridge's replacement-method
+dispatch takes a blocking `GMutex` on ART's hot dispatch paths, which
+deadlocks against ART thread suspension (GC, thread flips) on arm64.
+
+## What you need
+
+- arm64 Android 13 or 14 emulator (e.g. Cuttlefish, or an arm64 AVD)
+- frida >= 17 and frida-server for the same version
+- Node.js with `frida-compile`
+- Java + `d8` (Android build-tools) — optional, only for the worker dex
+
+## Steps
+
+```sh
+# 1. Build and push frida-server
+adb push frida-server /data/local/tmp/
+adb shell "/data/local/tmp/frida-server &"
+
+# 2. Disable the JIT (optional but recommended: with the JIT on, the
+#    workload can instead trip the dispatch-trampoline patch race and
+#    crash; the deadlock itself fires with the JIT on or off)
+adb root
+adb shell "setprop dalvik.vm.usejit false && stop && start"
+
+# 3. Bundle the script against the frida-java-bridge build under test
+npm install frida-java-bridge frida-compile   # released npm = unfixed
+npx frida-compile repro-deadlock-entry.js -o repro-bundled.js
+adb push repro-bundled.js /data/local/tmp/
+
+# 4. Run it against any app (Settings works fine)
+frida -U -f com.android.settings -l /data/local/tmp/repro-bundled.js
+```
+
+The script hooks frequently-called native methods (forwarding
+implementations) and prints a heartbeat every 30 s that includes a Java
+round-trip latency probe.
+
+## Expected results
+
+**Unfixed bridge (any released frida-java-bridge):** the process wedges
+within minutes. The heartbeat stops (the JS thread's next Java call parks
+on the dispatch mutex) and the app ANRs. Confirm the signature with
+`adb shell kill -3 <pid>` and inspect the newest `/data/anr/anr_*`
+(or `debuggerd -b <pid>`): several threads blocked in
+`pthread_mutex_lock` (bionic `NonPI::MutexLockWithTimeout`) at the SAME
+PC in the dispatch trampoline's code, entered from
+`art_quick_generic_jni_trampoline`.
+
+**Fixed bridge (this branch):** no dispatch-mutex cluster appears, the
+app's threads stay responsive, and the run completes without a wedge.
+
+## Notes
+
+- The deadlock needs concurrent JNI crossing + GC/flip activity. On an
+  idle app the crossing volume comes from the framework's constant calls
+  to the hooked time/id methods; if your device is very quiet, bump the
+  crossing rate by opening the app's settings pages or adding the
+  optional worker dex (see `ReproWorker.java`).
+- Do not drive GCs from the script's own thread: a direct
+  `Runtime.gc()` (or heavy allocation) from the frida JS thread wedges
+  that thread against ART's GC-completion wait even with no hooks at
+  all, which confounds the repro.
+- On fast emulators with the JIT enabled the workload can instead trip
+  the trampoline-prologue patch race (crash) — the deadlock is the
+  failure mode on slower devices and with the JIT off, which is also
+  what the ANR-trace signature distinguishes.

repro/ReproWorker.java

@@ -0,0 +1,38 @@
+/*
+ * Repro worker: real Java threads that hammer the instrumented JNI
+ * boundary while driving GC pressure. Loaded via frida's
+ * Java.openClassFile() from /data/local/tmp/repro-worker.dex (built from
+ * ReproWorker.java; see README.md).
+ */
+package com.example.repro;
+
+public final class ReproWorker implements Runnable {
+    private static final long RUN_NANOS = 10L * 60L * 1000L * 1000L * 1000L;
+    private static final int GC_INTERVAL = 4096;
+    private static final int BATCH = 64;
+    private static final int SLOT_SIZE = 4096;
+
+    public void run() {
+        final long end = System.nanoTime() + RUN_NANOS;
+        final Runtime rt = Runtime.getRuntime();
+        final Object marker = new Object();
+        final byte[][] slots = new byte[4096][];
+        int n = 0;
+        int counter = 0;
+        while (System.nanoTime() < end) {
+            // Every one of these is a hooked native method, so each call
+            // crosses the patched quick entrypoints (and, on the unfixed
+            // bridge, the dispatch GMutex).
+            System.currentTimeMillis();
+            System.nanoTime();
+            System.identityHashCode(marker);
+            Thread.currentThread();
+            for (int i = 0; i < BATCH; i++) {
+                slots[n++ % slots.length] = new byte[SLOT_SIZE];
+            }
+            if ((counter++ & (GC_INTERVAL - 1)) == 0) {
+                rt.gc();
+            }
+        }
+    }
+}

repro/repro-deadlock-entry.js

@@ -0,0 +1,12 @@
+/*
+ * Bundled variant of repro-deadlock.js for testing a specific
+ * frida-java-bridge build (e.g. this branch's fix).
+ *
+ *   npm install frida-java-bridge   # from this branch, or a local path
+ *   npx frida-compile repro-deadlock-entry.js -o repro-bundled.js
+ *   frida -U -f com.android.settings -l repro-bundled.js
+ */
+const mod = require('frida-java-bridge');
+const Java = mod.default ?? mod;
+globalThis.Java = Java;
+require('./repro-deadlock.js');