fix: attach the logger thread on android - #628
Merged
Merged
Conversation
`JavaVM::AttachCurrentThread` takes a `JNIEnv **` on android and a `void **`
on the jdk, and neither pointer converts to the other, so the `void **` cast
`ScopedEnv` used broke the android build outright:
jni/src/jni_logger.cpp:31:37: error: cannot initialize a parameter of type
'JNIEnv **' with an rvalue of type 'void **'
The branch lives in a small `attach_current_thread` helper, so the call site
reads like a plain JNI call and there is one place to look when a jni.h moves
again. `GetEnv` right above it is `void **` on both and stays as it was.
Deducing the pointer type from `&JavaVM::AttachCurrentThread` compiles on both
platforms without the macro, but a template around a two line call reads worse
than the branch it replaces.
Nothing in the bindings reaches the attach path yet: the one native taking a
logger, `Odr.open`, runs on the calling java thread, and the reaper that runs
`~JavaLogger` is a java thread too. `HttpServer` logs from cpp-httplib worker
threads though, so the attach is what keeps those lines from silently vanishing
once it takes a logger.
Found building odrcore 6.0.0 for OpenDocument.droid, which is the only consumer
that cross compiles this for android - the jni workflow builds host side only,
where the jdk declaration compiles.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YVri5Ct8L6JYa6ce2YWTSJ
andiwand
force-pushed
the
fix/jni-android-attach-current-thread
branch
from
July 27, 2026 22:49
7043fd7 to
e4ba1e3
Compare
andiwand
added a commit
to opendocument-app/OpenDocument.droid
that referenced
this pull request
Jul 27, 2026
The index commit adds an odrcore/6 recipe and pins 6.0.1: 6.0.0 drops the pdf2htmlEX and wvWare backends, so `with_pdf2htmlEX` and `with_wvWare` no longer exist as options and the two `=False` lines have to go with them - the build is exactly what it already was, the options just stopped being the way to ask for it. 6.0.1 is that release plus the android build fix in opendocument-app/OpenDocument.core#628, without which `libodr_jni.so` does not compile for any ABI. Two things the app talks to went with that release: - `DecoderEngine` is gone entirely. With the external backends removed the enum had one value left and could not select anything, so `open()` drops the `DecodePreference` that named it. It only ever existed to keep the engines that are not compiled in from being offered. - `HttpServer` splits `listen(host, port)` into `bind(host, port)` and `listen()`, and `Config.cachePath` is gone with `serveFile` - what a service was translated into belongs to whoever translated it, and this app translates through `Html.translate` and hands the cache path there already. `bind` is what the port dance was missing. It returns the port it actually got and throws when it cannot bind, which is what the ServerSocket probe in front of `listen` was approximating - so the probe goes, along with the reasoning about which socket options it had to match. The core sets SO_REUSEADDR now rather than cpp-httplib's SO_REUSEPORT, so a port held only by TIME_WAIT sockets is bindable again and only a live server of the other flavor still sends us to a free one. Verified with the four ABIs built against odrcore 6: unit tests, lint, and the 12 instrumented tests on a Pixel 6 Pro AVD, which load and edit real documents off that server. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015CX6aCCt9pM9WaTF5MwM5o
andiwand
added a commit
to opendocument-app/OpenDocument.droid
that referenced
this pull request
Jul 27, 2026
…544) The index commit adds an odrcore/6 recipe and pins 6.0.1: 6.0.0 drops the pdf2htmlEX and wvWare backends, so `with_pdf2htmlEX` and `with_wvWare` no longer exist as options and the two `=False` lines have to go with them - the build is exactly what it already was, the options just stopped being the way to ask for it. 6.0.1 is that release plus the android build fix in opendocument-app/OpenDocument.core#628, without which `libodr_jni.so` does not compile for any ABI. Two things the app talks to went with that release: - `DecoderEngine` is gone entirely. With the external backends removed the enum had one value left and could not select anything, so `open()` drops the `DecodePreference` that named it. It only ever existed to keep the engines that are not compiled in from being offered. - `HttpServer` splits `listen(host, port)` into `bind(host, port)` and `listen()`, and `Config.cachePath` is gone with `serveFile` - what a service was translated into belongs to whoever translated it, and this app translates through `Html.translate` and hands the cache path there already. `bind` is what the port dance was missing. It returns the port it actually got and throws when it cannot bind, which is what the ServerSocket probe in front of `listen` was approximating - so the probe goes, along with the reasoning about which socket options it had to match. The core sets SO_REUSEADDR now rather than cpp-httplib's SO_REUSEPORT, so a port held only by TIME_WAIT sockets is bindable again and only a live server of the other flavor still sends us to a free one. Verified with the four ABIs built against odrcore 6: unit tests, lint, and the 12 instrumented tests on a Pixel 6 Pro AVD, which load and edit real documents off that server. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
andiwand
added a commit
that referenced
this pull request
Jul 28, 2026
…ce (#629) * feat(android): package the bindings as an AAR and test them on a device Two android-only failure modes shipped in a row: #628, a compile error in `jni/src` that only the NDK sees, and #621, a `java.lang.ref.Cleaner` that exists on the JDK and not on android's class library. Both were found by OpenDocument.droid, because it was the only thing that ever cross compiled this, and the build_test matrix's single android entry only ever configured conan for one ABI — it never built the bindings, let alone ran them. `android/` closes that: a gradle library module that packages the JNI bindings as `app.opendocument:odr-core-android`, and `.github/workflows/ android.yml`, which on every push cross compiles all four ABIs, lints `../jni/java` against minSdk 26, assembles the AAR, and runs an instrumented suite on API 26 (the floor OpenDocument.droid ships to) and on API 36. The AAR carries `classes.jar` (the same `../jni/java` the maven jar is built from, plus `OdrAndroid`), `libodr_jni.so` and `libc++_shared.so` per ABI, the renderer's CSS/JS and the libmagic database as assets, and a consumer proguard rule keeping the classes JNI resolves by name. `OdrAndroid.init(context)` unpacks the assets into no-backup storage, keyed by `Odr.commitHash()`, and registers them with `GlobalParams` — an APK holds them as assets, and the renderer wants files. `build_native.py` drives conan + cmake per ABI into `native/prebuilt`, where the gradle build reads them; CI runs it once per ABI as a separate job and hands the results to the AAR and emulator jobs as artifacts. This is a second packaging of the same build, not a new path: OpenDocument.droid keeps consuming the conan package (`with_jni=True`), which needs no credentials, as f-droid requires. The AAR is for consumers that just want a dependency. Along the way: * The four `android-<arch>` conan profiles replace `android-35-x86_64` and share one `android.jinja` body. API level drops 35 -> 26 to match the java floor (it is baked into the target triple, so a higher one produces libraries older devices refuse to load) and the NDK toolchain path is resolved per host, so the profiles work on macOS too. `scripts/conan_lock` locks each of them against the linux build profile. * `jni/CMakeLists.txt` builds `odr_jni` without a JDK when targeting android: the NDK sysroot ships `jni.h` and the symbols come from the runtime. A JDK found there still builds the jar, which is what the conan package deploys next to the library; the junit suite stays host-only. * `TestFiles` moves to `jni/testfixtures`, compiled by both suites, so it is limited to what API 26 offers — `String.formatted`, `Path.of` and `Files.writeString` are gone from it. * `Color.toString` formats with `Locale.ROOT`; a device set to a locale with its own digits rendered it unparseable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(android)!: write the AAR's own code in kotlin OpenDocument.droid is kotlin throughout, and it is the consumer this AAR is built for, so the module it will depend on should not be the odd one out. The two things this module writes itself move over: `OdrAndroid` and the instrumented suite. What it *borrows* stays java, and has to. `../jni/java` and `../jni/testfixtures` are compiled by CMake's `add_jar` for the maven jar and the host junit suite, neither of which has a kotlin toolchain — so the java API is unchanged, and `src/androidTest` compiles kotlin against the java `TestFiles` in the same package, which resolves the package-private members fine. The public surface keeps its java shape: `OdrAndroid` is an `object` with `@JvmStatic init`, so `OdrAndroid.init(context)` is still a static call, and `@Throws(IOException::class)` keeps the exception checked for a java caller. Kotlin earns its place mostly by deleting code: `File.deleteRecursively`, `InputStream.copyTo` and `use` replace three hand-rolled helpers in `OdrAndroid`, and `readBytes()` replaces the buffer loop in `HttpServerTest`. Net -76 lines across the six files, with the same tests. BREAKING CHANGE: `odr-core-android` now declares a `kotlin-stdlib` dependency (2.2.10, AGP 9's built-in kotlin). The POM had none before, which is what `android.builtInKotlin=false` was protecting; for OpenDocument.droid it costs nothing, and it is the price of the module being kotlin at all. Nothing about `odr-core-java` changes — that jar is still pure java with no dependencies. Also adds spotless + ktfmt 0.64 in the kotlinlang flavor, the same formatter and version OpenDocument.droid runs, scoped to this module's own kotlin so an android build never rewrites the maven jar's sources. `spotlessCheck` runs in the format workflow, which needs neither the NDK nor conan. Verified on an API 31 emulator: 16/16 instrumented tests pass, lint is clean against minSdk 26, and the release AAR assembles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(android): store the gradle wrapper jar unmangled, gate publishing on the device suite `.gitattributes` declares `* text eol=lf`, so committing the wrapper jar normalized the two CR bytes out of it: the blob is 48460 bytes where the jar gradle publishes is 48462, and `setup-gradle`'s wrapper validation rejects the checkout outright. Declare `*.jar binary` and renormalize; the blob now hashes to gradle's published 497c8c2a7e5031f6aa847f88104aa80a93532ec32ee17bdb8d1d2f67a194a9c7. Publishing moves out of `aar` into its own job needing both `aar` and `instrumented`. As siblings off `native`, `aar` could push a release to GitHub Packages before — or despite — the emulator suite, which is the one check that sees what a device sees. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
JavaVM::AttachCurrentThreadtakes aJNIEnv **on android and avoid **on the jdk, and neither pointer converts to the other, so thevoid **cast inScopedEnvbreaks the android build outright:GetEnvright above it isvoid **on both and stays as it was. Nothing else in the bindings attaches a thread.Found building odrcore 6.0.0 for OpenDocument.droid, which is the only consumer that cross compiles the bindings for android —
.github/workflows/jni.ymlbuilds host side only, where the jdk declaration compiles and this stays invisible.Test
cmake --build cmake-build-relwithdebinfo --target odr_jni(macOS/jdk) — passes, as before.armv8,armv7,x86andx86_64withwith_jni=True, NDK 28.2.13676358, api 26:libodr_jni.solinks for all four. Before this it failed atjni_logger.cppon every one of them.