From e13bcb45a9f6fac5c92025da0c5f2a5382fd129c Mon Sep 17 00:00:00 2001
From: Rob Pitkin <[email protected]>
Date: Thu, 08 Oct 2026 10:47:11 -0700
Subject: [PATCH] shape-detection: Respond to Detect() on the binding thread

On Android, FaceDetectionImpl.detect() posted face detection to a
ThreadPool worker and invoked the Mojo response callback from that
worker. The Java Mojo bindings (Connector, HandleBase) are not
thread-safe. If the remote end closed the pipe while the worker was
responding, the binding thread closed the same handle concurrently,
causing a use-after-free or double close of the native message pipe in
the GPU process.

Capture the binding thread's executor via ExecutorFactory before
posting the work, and send the response through it, so all access to
the pipe happens on the binding thread. This matches
AndroidFontLookupImpl.

Bug: 568899705
Change-Id: I24f789e8e254ed135674b496618db176139ff99d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8504201
Reviewed-by: Alvin Ji <[email protected]>
Commit-Queue: Rob Pitkin <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1714708}
---
diff --git a/services/shape_detection/android/java/src/org/chromium/shape_detection/FaceDetectionImpl.java b/services/shape_detection/android/java/src/org/chromium/shape_detection/FaceDetectionImpl.java
index 190a45c1..1540d5f 100644
--- a/services/shape_detection/android/java/src/org/chromium/shape_detection/FaceDetectionImpl.java
+++ b/services/shape_detection/android/java/src/org/chromium/shape_detection/FaceDetectionImpl.java
@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 package org.chromium.shape_detection;
-import org.chromium.build.annotations.NullMarked;
 
 import android.graphics.Bitmap;
 import android.graphics.Canvas;
@@ -14,13 +13,18 @@
 import org.chromium.base.Log;
 import org.chromium.base.task.PostTask;
 import org.chromium.base.task.TaskTraits;
+import org.chromium.build.annotations.NullMarked;
 import org.chromium.gfx.mojom.RectF;
+import org.chromium.mojo.bindings.ExecutorFactory;
 import org.chromium.mojo.system.MojoException;
+import org.chromium.mojo.system.impl.CoreImpl;
 import org.chromium.shape_detection.mojom.FaceDetection;
 import org.chromium.shape_detection.mojom.FaceDetectionResult;
 import org.chromium.shape_detection.mojom.FaceDetectorOptions;
 import org.chromium.shape_detection.mojom.Landmark;
 
+import java.util.concurrent.Executor;
+
 /**
  * Android implementation of the FaceDetection service defined in
  * services/shape_detection/public/mojom/facedetection.mojom
@@ -71,6 +75,11 @@
         final Bitmap unPremultipliedBitmap =
                 Bitmap.createBitmap(pixels, width, height, Bitmap.Config.RGB_565);
 
+        // Java Mojo bindings are not thread-safe. Calling `callback` from the worker races the
+        // binding thread closing the pipe when the peer disconnects, which can cause a
+        // use-after-free. Capture the binding thread's executor so the response is sent from it.
+        final Executor bindingThreadExecutor = getResponseExecutor();
+
         // FaceDetector creation and findFaces() might take a long time and trigger a
         // "StrictMode policy violation": they should happen in a background thread.
         PostTask.postTask(
@@ -102,10 +111,15 @@
                         faceArray[i].landmarks = new Landmark[0];
                     }
 
-                    callback.call(faceArray);
+                    bindingThreadExecutor.execute(() -> callback.call(faceArray));
                 });
     }
 
+    /** Returns an executor that runs tasks on the calling thread. Overridden in tests. */
+    Executor getResponseExecutor() {
+        return ExecutorFactory.getExecutorForCurrentThread(CoreImpl.getInstance());
+    }
+
     @Override
     public void close() {}
 
diff --git a/services/shape_detection/android/javatests/src/org/chromium/shape_detection/FaceDetectionImplTest.java b/services/shape_detection/android/javatests/src/org/chromium/shape_detection/FaceDetectionImplTest.java
index 7a60f0f..36aaecc9 100644
--- a/services/shape_detection/android/javatests/src/org/chromium/shape_detection/FaceDetectionImplTest.java
+++ b/services/shape_detection/android/javatests/src/org/chromium/shape_detection/FaceDetectionImplTest.java
@@ -28,7 +28,9 @@
 import java.util.Arrays;
 import java.util.Comparator;
 import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 
 /** Test suite for FaceDetectionImpl. */
 @RunWith(BaseJUnit4ClassRunner.class)
@@ -64,32 +66,51 @@
         FaceDetectorOptions options = new FaceDetectorOptions();
         options.fastMode = fastMode;
         options.maxDetectedFaces = 32;
+        // Tasks the impl hands to its response executor; run below on this thread, as the
+        // binding thread would in production.
+        final ArrayBlockingQueue<Runnable> responseTasks = new ArrayBlockingQueue<>(1);
+        final AtomicReference<Thread> executorThread = new AtomicReference<>();
         FaceDetection detector = null;
         if (api == DetectionProviderType.ANDROID) {
-            detector = new FaceDetectionImpl(options);
+            detector =
+                    new FaceDetectionImpl(options) {
+                        @Override
+                        Executor getResponseExecutor() {
+                            executorThread.set(Thread.currentThread());
+                            return responseTasks::add;
+                        }
+                    };
         } else if (api == DetectionProviderType.GMS_CORE) {
             detector = new FaceDetectionImplGmsCore(options);
         } else {
             throw new AssertionError();
         }
 
-        final ArrayBlockingQueue<FaceDetectionResult[]> queue = new ArrayBlockingQueue<>(1);
+        final AtomicReference<FaceDetectionResult[]> result = new AtomicReference<>();
+        final AtomicReference<Thread> callbackThread = new AtomicReference<>();
         detector.detect(
                 mojoBitmap,
-                new FaceDetection.Detect_Response() {
-                    @Override
-                    public void call(FaceDetectionResult[] results) {
-                        queue.add(results);
-                    }
+                results -> {
+                    callbackThread.set(Thread.currentThread());
+                    result.set(results);
                 });
-        FaceDetectionResult[] toReturn = null;
-        try {
-            toReturn = queue.poll(5L, TimeUnit.SECONDS);
-        } catch (InterruptedException e) {
-            Assert.fail("Could not get FaceDetectionResult: " + e.toString());
+        if (result.get() == null) {
+            Runnable responseTask = null;
+            try {
+                responseTask = responseTasks.poll(5L, TimeUnit.SECONDS);
+            } catch (InterruptedException e) {
+                Assert.fail("Could not get FaceDetectionResult: " + e.toString());
+            }
+            Assert.assertNotNull(responseTask);
+            responseTask.run();
         }
-        Assert.assertNotNull(toReturn);
-        return toReturn;
+        Assert.assertNotNull(result.get());
+        // Mojo responses must be sent from the binding thread (https://crbug.com/568899705).
+        if (api == DetectionProviderType.ANDROID) {
+            Assert.assertEquals(Thread.currentThread(), executorThread.get());
+        }
+        Assert.assertEquals(Thread.currentThread(), callbackThread.get());
+        return result.get();
     }
 
     private void detectSucceedsOnValidImage(DetectionProviderType api) {