fix(jni): keep a wrapper reachable while its handle is in a native call

jni/AGENTS.md

@@ -29,9 +29,20 @@ package `app.opendocument.core`. Mirrors the surface of the python bindings
 - **Keep-alive**: navigation results carry an `owner` reference
   (`Element.owner()` → the `Document`) so the GC cannot free the root while
   handles into it are alive — the JNI analogue of pyodr's `keep_alive`.
-- **GC safety**: natives that use a handle are *instance* methods (the `this`
-  local reference keeps the wrapper — and its owner chain — reachable for the
-  duration of the call); only factories and `destroy` are static.
+- **GC safety**: natives that use a handle are *instance* methods **of the
+  object that owns it** (the `this` local reference keeps the wrapper — and its
+  owner chain — reachable for the duration of the call); only factories and
+  `destroy` are static. A static native taking someone else's handle is a bug
+  even where it reads fine: once `handle()` has returned nothing refers to the
+  wrapper, so the collector may enqueue it and the reaper free the handle while
+  the call is still running. Put the native on the owner and let the entry point
+  delegate — `Html.edit` → `Document.edit`, `new DecodedFile(file)` →
+  `File.decode`, and `Logger`'s own natives.
+- **Handles in argument position** — `a.fooNative(handle(), b.handle())` — have
+  no receiver holding them, so `b` needs `NativeResource.keepAlive()` in a
+  `finally` after the call. That is what `Reference.reachabilityFence` is for;
+  android only has it from API 28 (see the API floor below), so `keepAlive` is
+  the JDK's own pre-intrinsic fallback instead.
 - **Enums cross as ordinals**: Java enum constant order must match the C++
   enum declaration; `-1` encodes an absent `std::optional`.
 - **Strings**: use `odr_jni::to_string`/`to_jstring` (real UTF-8 ↔ UTF-16),
@@ -60,7 +71,9 @@ package `app.opendocument.core`. Mirrors the surface of the python bindings
   `--release 17` compiler accepts. Anything newer fails at runtime, on device
   only, with `NoClassDefFoundError`/`NoSuchMethodError`. Known traps, all of
   which android added far later than the JDK: `java.lang.ref.Cleaner` (API 33,
-  hence the `PhantomReference` reaper), `List.of`/`Set.of`/`Map.of` (API 34),
+  hence the `PhantomReference` reaper),
+  `java.lang.ref.Reference.reachabilityFence` (API 28, hence `keepAlive`),
+  `List.of`/`Set.of`/`Map.of` (API 34),
   `Optional.isEmpty` (API 33), `java.time` (API 26 only in part). Core library
   desugaring does not cover `java.lang.ref`, and an app cannot shim a `java.*`
   class, so the fix always has to happen here. This is no longer only a rule to

jni/java/app/opendocument/core/DecodedFile.java

@@ -23,7 +23,7 @@ public DecodedFile(String path, FileType as) {
   }
 
   public DecodedFile(File file) {
-    this(createFromFile(file.handle()));
+    this(file.decode());
   }
 
   public File file() {
@@ -120,8 +120,6 @@ public FontFile asFontFile() {
 
   private static native long createAs(String path, int as);
 
-  private static native long createFromFile(long fileHandle);
-
   static native void destroy(long handle);
 
   private native long fileNative(long handle);

jni/java/app/opendocument/core/Document.java

@@ -43,8 +43,15 @@ public Filesystem asFilesystem() {
     return new Filesystem(asFilesystemNative(handle()), this);
   }
 
+  /** Applies a diff; what {@link Html#edit} calls. */
+  void edit(String diff) {
+    editNative(handle(), diff);
+  }
+
   private static native void destroy(long handle);
 
+  private native void editNative(long handle, String diff);
+
   private native boolean isEditableNative(long handle);
 
   private native boolean isSavableNative(long handle, boolean encrypted);

jni/java/app/opendocument/core/DocumentPath.java

@@ -23,7 +23,11 @@ public DocumentPath parent() {
   }
 
   public DocumentPath join(DocumentPath other) {
-    return new DocumentPath(joinNative(handle(), other.handle()));
+    try {
+      return new DocumentPath(joinNative(handle(), other.handle()));
+    } finally {
+      other.keepAlive();
+    }
   }
 
   @Override

jni/java/app/opendocument/core/Element.java

@@ -49,15 +49,23 @@ public boolean isEditable() {
 
   /** Whether this and {@code other} refer to the same element. */
   public boolean isSame(Element other) {
-    return isSameNative(handle(), other.handle());
+    try {
+      return isSameNative(handle(), other.handle());
+    } finally {
+      other.keepAlive();
+    }
   }
 
   public DocumentPath documentPath() {
     return new DocumentPath(documentPathNative(handle()));
   }
 
   public Element navigatePath(DocumentPath path) {
-    return wrap(navigatePathNative(handle(), path.handle()));
+    try {
+      return wrap(navigatePathNative(handle(), path.handle()));
+    } finally {
+      path.keepAlive();
+    }
   }
 
   public List<Element> children() {

jni/java/app/opendocument/core/File.java

@@ -36,10 +36,17 @@ public void copy(String path) {
     copyNative(handle(), path);
   }
 
+  /** Decodes this file; the handle for {@link DecodedFile#DecodedFile(File)}. */
+  long decode() {
+    return decodeNative(handle());
+  }
+
   private static native long create(String path);
 
   private static native void destroy(long handle);
 
+  private native long decodeNative(long handle);
+
   private native int locationNative(long handle);
 
   private native long sizeNative(long handle);

jni/java/app/opendocument/core/Html.java

@@ -68,7 +68,7 @@ public static HtmlService translate(Filesystem filesystem, String cachePath, Htm
 
   /** Applies a diff (produced by the browser-side editor) to a document. */
   public static void edit(Document document, String diff) {
-    editDocument(document.handle(), diff);
+    document.edit(diff);
   }
 
   private static native long translateFile(long fileHandle, String cachePath, HtmlConfig config);
@@ -78,6 +78,4 @@ private static native long translateDocument(
 
   private static native long translateFilesystem(
       long filesystemHandle, String cachePath, HtmlConfig config);
-
-  private static native void editDocument(long documentHandle, String diff);
 }

jni/java/app/opendocument/core/HttpServer.java

@@ -40,7 +40,11 @@ public HttpServer(Config config) {
 
   /** Hosts the service under {@code /<prefix>/<view path>}. */
   public void connectService(HtmlService service, String prefix) {
-    connectServiceNative(handle(), service.handle(), prefix);
+    try {
+      connectServiceNative(handle(), service.handle(), prefix);
+    } finally {
+      service.keepAlive();
+    }
   }
 
   /**

jni/java/app/opendocument/core/Logger.java

@@ -48,11 +48,11 @@ public void flush() {
 
   private static native long createFromSink(ILogger sink);
 
-  private static native boolean willLogNative(long handle, int level);
+  private native boolean willLogNative(long handle, int level);
 
-  private static native void logNative(long handle, int level, String message);
+  private native void logNative(long handle, int level, String message);
 
-  private static native void flushNative(long handle);
+  private native void flushNative(long handle);
 
   private static native void destroy(long handle);
 }

jni/java/app/opendocument/core/NativeResource.java

@@ -81,6 +81,32 @@ final boolean isClosed() {
     return closed;
   }
 
+  /**
+   * A use of this object that the optimiser cannot drop, so it stays reachable
+   * across whatever ran before it.
+   *
+   * <p>{@link #handle()} outlives the wrapper it came from: once nothing refers to
+   * the wrapper any more the collector may enqueue it and the reaper may free the
+   * handle, and the just-in-time compiler is free to decide that while a native
+   * call using it is still running. A native that takes the handle of the object it
+   * is called on is safe without this - the JNI frame holds the receiver - which is
+   * why every one of them is an instance method of its owner. This is for the
+   * handles that go in as arguments, where there is no receiver to hold them.
+   *
+   * <p>{@code java.lang.ref.Reference.reachabilityFence} is the API for it, and is
+   * unusable here: android has it from API level 28, {@code android/build.gradle.kts}
+   * sets {@code minSdk = 26}, and core library desugaring does not cover
+   * {@code java.lang.ref} - the same wall {@link java.lang.ref.Cleaner} hit above.
+   * Taking the monitor of an object the reference queue can also see is the fallback
+   * the JDK itself used before that method existed: lock elision needs the object to
+   * be provably confined, and this one is not.
+   */
+  final void keepAlive() {
+    synchronized (this) {
+      // empty on purpose - taking the monitor is the use
+    }
+  }
+
   /** Frees the native object early. Idempotent. */
   @Override
   public void close() {

jni/java/app/opendocument/core/Odr.java

@@ -107,7 +107,11 @@ public static DecodedFile open(String path) {
 
   /** Opens and decodes a file, reporting diagnostics to {@code logger}. */
   public static DecodedFile open(String path, Logger logger) {
-    return new DecodedFile(openWithLoggerNative(path, logger.handle()));
+    try {
+      return new DecodedFile(openWithLoggerNative(path, logger.handle()));
+    } finally {
+      logger.keepAlive();
+    }
   }
 
   /** Opens and decodes a file as the given file type. */

jni/src/jni_document.cpp

@@ -5,6 +5,7 @@
 #include <odr/document_element.hpp>
 #include <odr/document_path.hpp>
 #include <odr/filesystem.hpp>
+#include <odr/html.hpp>
 
 #include <vector>
 
@@ -53,6 +54,17 @@ Java_app_opendocument_core_Document_destroy(JNIEnv *, jclass, jlong handle) {
   destroy_handle<odr::Document>(handle);
 }
 
+// odr::html::edit, but it belongs to Document: a native that takes a handle has
+// to be an instance method of whatever owns it, or the wrapper can be collected
+// - and the handle freed - while the call is still running
+extern "C" JNIEXPORT void JNICALL
+Java_app_opendocument_core_Document_editNative(JNIEnv *env, jobject,
+                                               jlong handle, jstring diff) {
+  guarded(env, [&] {
+    odr::html::edit(*from_handle<odr::Document>(handle), to_string(env, diff));
+  });
+}
+
 extern "C" JNIEXPORT jboolean JNICALL
 Java_app_opendocument_core_Document_isEditableNative(JNIEnv *env, jobject,
                                                      jlong handle) {

jni/src/jni_file.cpp

@@ -76,6 +76,16 @@ extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_File_copyNative(
           [&] { from_handle<odr::File>(handle)->copy(to_string(env, path)); });
 }
 
+// yields a DecodedFile but belongs to File: a native that takes a handle has to
+// be an instance method of whatever owns it, or the wrapper can be collected -
+// and the handle freed - while the call is still running
+extern "C" JNIEXPORT jlong JNICALL Java_app_opendocument_core_File_decodeNative(
+    JNIEnv *env, jobject, jlong handle) {
+  return guarded(env, [&] {
+    return make_handle(odr::DecodedFile(*from_handle<odr::File>(handle)));
+  });
+}
+
 // app.opendocument.core.DecodedFile
 //
 // Handles hold a heap `odr::DecodedFile` (the typed C++ subclasses are sliced
@@ -97,14 +107,6 @@ Java_app_opendocument_core_DecodedFile_createAs(JNIEnv *env, jclass,
   });
 }
 
-extern "C" JNIEXPORT jlong JNICALL
-Java_app_opendocument_core_DecodedFile_createFromFile(JNIEnv *env, jclass,
-                                                      jlong file_handle) {
-  return guarded(env, [&] {
-    return make_handle(odr::DecodedFile(*from_handle<odr::File>(file_handle)));
-  });
-}
-
 extern "C" JNIEXPORT void JNICALL
 Java_app_opendocument_core_DecodedFile_destroy(JNIEnv *, jclass, jlong handle) {
   destroy_handle<odr::DecodedFile>(handle);

jni/src/jni_html.cpp

@@ -171,14 +171,6 @@ Java_app_opendocument_core_Html_translateFilesystem(JNIEnv *env, jclass,
   });
 }
 
-extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_Html_editDocument(
-    JNIEnv *env, jclass, jlong document_handle, jstring diff) {
-  guarded(env, [&] {
-    odr::html::edit(*from_handle<odr::Document>(document_handle),
-                    to_string(env, diff));
-  });
-}
-
 // app.opendocument.core.HtmlService
 
 extern "C" JNIEXPORT void JNICALL

jni/src/jni_logger.cpp

@@ -198,7 +198,7 @@ Java_app_opendocument_core_Logger_createFromSink(JNIEnv *env, jclass,
 }
 
 extern "C" JNIEXPORT jboolean JNICALL
-Java_app_opendocument_core_Logger_willLogNative(JNIEnv *env, jclass,
+Java_app_opendocument_core_Logger_willLogNative(JNIEnv *env, jobject,
                                                 jlong handle, jint level) {
   return guarded(env, [&] {
     return static_cast<jboolean>(from_handle<odr::Logger>(handle)->will_log(
@@ -207,15 +207,15 @@ Java_app_opendocument_core_Logger_willLogNative(JNIEnv *env, jclass,
 }
 
 extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_Logger_logNative(
-    JNIEnv *env, jclass, jlong handle, jint level, jstring message) {
+    JNIEnv *env, jobject, jlong handle, jint level, jstring message) {
   guarded(env, [&] {
     from_handle<odr::Logger>(handle)->log(static_cast<odr::LogLevel>(level),
                                           to_string(env, message));
   });
 }
 
 extern "C" JNIEXPORT void JNICALL Java_app_opendocument_core_Logger_flushNative(
-    JNIEnv *env, jclass, jlong handle) {
+    JNIEnv *env, jobject, jlong handle) {
   guarded(env, [&] { from_handle<odr::Logger>(handle)->flush(); });
 }
 

jni/tests/app/opendocument/core/DocumentTest.java

@@ -85,5 +85,24 @@ void documentPath() throws IOException {
     DocumentPath path = first.documentPath();
     assertNotNull(path.toString());
     assertEquals(path, first.documentPath());
+
+    // join() and navigatePath() take another wrapper's handle as an argument
+    DocumentPath rejoined = path.parent().join(path);
+    assertTrue(path.parent().empty());
+    assertEquals(path, rejoined);
+    assertTrue(document.rootElement().navigatePath(rejoined).isSame(first));
+  }
+
+  @Test
+  void editAppliesADiff() throws IOException {
+    Document document = openDocument();
+    assertTrue(document.isEditable());
+
+    Element paragraph = document.rootElement().firstChild();
+    DocumentPath text = paragraph.firstChild().documentPath();
+
+    Html.edit(document, "{\"modifiedText\":{\"" + text + "\":\"edited by the diff\"}}");
+
+    assertTrue(walkText(document.rootElement()).contains("edited by the diff"));
   }
 }

jni/tests/app/opendocument/core/FileTest.java

@@ -58,6 +58,15 @@ void fileReadMatchesSize() throws IOException {
     }
   }
 
+  @Test
+  void decodeAnOpenFile() throws IOException {
+    Path odt = TestFiles.odtFile(tempDir);
+    try (File file = new File(odt.toString());
+        DecodedFile decoded = new DecodedFile(file)) {
+      assertEquals(FileType.OPENDOCUMENT_TEXT, decoded.fileType());
+    }
+  }
+
   @Test
   void fileMeta() throws IOException {
     Path odt = TestFiles.odtFile(tempDir);