diff --git a/android_webview/js_sandbox/java/src/org/chromium/android_webview/js_sandbox/service/JsSandboxMessagePort.java b/android_webview/js_sandbox/java/src/org/chromium/android_webview/js_sandbox/service/JsSandboxMessagePort.java --- a/android_webview/js_sandbox/java/src/org/chromium/android_webview/js_sandbox/service/JsSandboxMessagePort.java +++ b/android_webview/js_sandbox/java/src/org/chromium/android_webview/js_sandbox/service/JsSandboxMessagePort.java @@ -12,11 +12,18 @@ import org.jni_zero.JniType; import org.jni_zero.NativeMethods; import org.chromium.build.annotations.NullMarked; +import javax.annotation.concurrent.GuardedBy; @JNINamespace("android_webview") @NullMarked public class JsSandboxMessagePort { - private final long mNativeJsSandboxMessagePort; + // The native pointer is a cppgc-heap address inside the V8 isolate's + // CppHeap. It is freed (without going through this class) when the isolate + // is torn down, so all dereferences must be serialised against close(). + private final Object mLock = new Object(); + @GuardedBy("mLock") + private long mNativeJsSandboxMessagePort; private final MessagePortInternal mMessagePortInternal; @CalledByNative @@ -37,29 +44,33 @@ public class JsSandboxMessagePort { new MessagePortInternal.MessagePortClient() { @Override public void onString(String string) { - // Note that the message may not always be stored as UTF-16. When stored in - // UTF-8, it may end up being more or less. As such, this is somewhat more - // of a best-effort estimate than a concrete value. - long size = (long) string.length() * Character.BYTES; - if (!JsSandboxMessagePortJni.get() - .tryAllocateMemoryBudget(mNativeJsSandboxMessagePort, size)) { - return; - } + synchronized (mLock) { + if (mNativeJsSandboxMessagePort == 0) return; + // Note that the message may not always be stored as UTF-16. When + // stored in UTF-8, it may end up being more or less. As such, this is + // somewhat more of a best-effort estimate than a concrete value. + long size = (long) string.length() * Character.BYTES; + if (!JsSandboxMessagePortJni.get() + .tryAllocateMemoryBudget(mNativeJsSandboxMessagePort, size)) { + return; + } - JsSandboxMessagePortJni.get() - .handleString(mNativeJsSandboxMessagePort, string, size); + JsSandboxMessagePortJni.get() + .handleString(mNativeJsSandboxMessagePort, string, size); + } } @Override public void onArrayBuffer(byte[] arrayBuffer) { - long size = arrayBuffer.length; - if (!JsSandboxMessagePortJni.get() - .tryAllocateMemoryBudget(mNativeJsSandboxMessagePort, size)) { - return; - } - - JsSandboxMessagePortJni.get() - .handleArrayBuffer(mNativeJsSandboxMessagePort, arrayBuffer, size); + synchronized (mLock) { + if (mNativeJsSandboxMessagePort == 0) return; + long size = arrayBuffer.length; + if (!JsSandboxMessagePortJni.get() + .tryAllocateMemoryBudget(mNativeJsSandboxMessagePort, size)) { + return; + } + JsSandboxMessagePortJni.get() + .handleArrayBuffer( + mNativeJsSandboxMessagePort, arrayBuffer, size); + } } }); mMessagePortInternal = messagePortInternal; @@ -77,9 +88,15 @@ public class JsSandboxMessagePort { mMessagePortInternal.postArrayBuffer(arrayBuffer); } - // Called by isolate thread + // Called by isolate thread from ~JsSandboxMessagePort() during CppHeap + // teardown. Taking mLock here guarantees that any in-flight JNI call on a + // Binder thread finishes before the native object is actually freed, and + // that no further JNI call will be attempted afterwards. @CalledByNative void close() { + synchronized (mLock) { + mNativeJsSandboxMessagePort = 0; + } mMessagePortInternal.close(); }