fix: resolve unsafe concurrency in Android JNI layer #261

jmanhype · 2025-11-27T08:52:04Z

Use LazyLock for safe static Mutex initialization instead of direct Mutex::new() which could have initialization races
Handle mutex poisoning gracefully by recovering instead of panicking on .unwrap() calls
Add double-start prevention to avoid spawning multiple server instances
Store handle before blocking to allow stopServer to work during startup
Release lock before blocking on readiness to avoid holding mutex during long waits
Replace eprintln with tracing macros for consistent logging
Add error codes for JNI callers to handle failures appropriately
Document concurrency safety guarantees in module docs

gemini-code-assist · 2025-11-27T08:52:23Z

Summary of Changes

Hello @jmanhype, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the robustness and concurrency safety of the Android JNI layer for the Forge server. It addresses potential race conditions and stability issues by implementing safe static mutex initialization, graceful handling of mutex poisoning, and mechanisms to prevent multiple server instances. Additionally, it improves the control flow during server startup and shutdown, standardizes logging, and provides clearer error reporting to the Android application.

Highlights

Safe Static Mutex Initialization: The SERVER_HANDLE static mutex is now initialized using LazyLock, ensuring thread-safe, one-time initialization and preventing potential initialization races.
Mutex Poisoning Recovery: Both startServer and stopServer functions now gracefully handle poisoned mutexes by recovering the inner value instead of panicking, improving application stability.
Double-Start Prevention: Logic has been added to startServer to detect if a server is already running and active, preventing the spawning of multiple server instances and returning an appropriate error code.
Improved Server Control during Startup: The server's JoinHandle is now stored in SERVER_HANDLE before startServer blocks on readiness, allowing stopServer to abort the server even during its initial startup phase.
Reduced Lock Contention: The mutex lock is explicitly released (drop(guard)) before startServer blocks on the server's readiness, preventing the mutex from being held unnecessarily during potentially long waits.
Standardized Logging: Direct eprintln! calls have been replaced with tracing macros (tracing::error!, tracing::warn!, tracing::info!, tracing::debug!) for consistent and structured logging across the module.
JNI Error Reporting: New JNI error codes (ERR_ALREADY_RUNNING, ERR_SERVER_START_FAILED) have been introduced, allowing Android callers to receive more specific feedback on server start failures.
Concurrency Safety Documentation: The module-level documentation for forge-app/src/android.rs has been updated to explicitly detail the concurrency safety guarantees provided by the JNI layer.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request significantly improves the concurrency safety of the Android JNI layer. The changes, such as using LazyLock for static initialization, handling mutex poisoning gracefully, preventing double server starts, and fixing a potential deadlock by releasing the mutex before blocking, are all excellent and make the code much more robust. The introduction of error codes for the JNI caller is also a great improvement for error handling on the client side. I have one minor suggestion to align the documentation with the implementation.

gemini-code-assist · 2025-11-27T08:53:10Z

forge-app/src/android.rs

+/// # Returns
+/// - Positive value: The port number the server is listening on
+/// - `-1` (`ERR_ALREADY_RUNNING`): Server is already running
+/// - `-2` (`ERR_LOCK_FAILED`): Failed to acquire lock (mutex poisoned)


The documentation states that ERR_LOCK_FAILED (-2) is returned if the mutex is poisoned. However, the implementation recovers from a poisoned mutex by calling poisoned.into_inner() and continues execution, rather than returning an error. This makes the documentation inconsistent with the code's behavior. To avoid confusion for the JNI caller, this line should be removed from the documentation, especially since the ERR_LOCK_FAILED constant is marked as Reserved for future use.

- Use LazyLock for safe static Mutex initialization instead of direct Mutex::new() which could have initialization races - Handle mutex poisoning gracefully by recovering instead of panicking on .unwrap() calls - Add double-start prevention to avoid spawning multiple server instances - Store handle before blocking to allow stopServer to work during startup - Release lock before blocking on readiness to avoid holding mutex during long waits - Replace eprintln with tracing macros for consistent logging - Add error codes for JNI callers to handle failures appropriately - Document concurrency safety guarantees in module docs Co-authored-by: Automagik Genie 🧞 <genie@namastex.ai>

namastex888 · 2025-11-28T23:10:29Z

@codex review this pr

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2025-11-28T23:13:40Z

forge-app/src/android.rs

 static RUNTIME: OnceLock<Runtime> = OnceLock::new();
-static SERVER_HANDLE: Mutex<Option<tokio::task::JoinHandle<()>>> = Mutex::new(None);
-static LAST_ERROR: Mutex<Option<String>> = Mutex::new(None);
+static SERVER_HANDLE: LazyLock<Mutex<Option<tokio::task::JoinHandle<()>>>> =
+    LazyLock::new(|| Mutex::new(None));


Restore JNI exports expected by Android app

android.rs now defines only the server handle and the start/stop JNI functions, removing the setDataDir/getLastError bindings that previously used LAST_ERROR. MainActivity.kt still declares those native methods (android/app/src/main/java/ai/namastex/forge/MainActivity.kt lines 23-27), so loading the library and invoking setDataDir during app startup will throw UnsatisfiedLinkError: Native method not found, preventing the Android app from running or reporting backend errors. The missing JNI exports need to be reinstated or the Kotlin declarations removed to avoid the crash.

Useful? React with 👍 / 👎.

jmanhype · 2025-11-29T02:40:39Z

@codex review this pr

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2025-11-29T02:43:00Z

forge-app/src/android.rs

+#[cfg(feature = "android")]
 #[unsafe(no_mangle)]
 pub extern "C" fn Java_ai_namastex_forge_MainActivity_startServer(
    _env: JNIEnv,


Restore removed JNI exports needed by MainActivity

The Android JNI module now only exports startServer/stopServer (lines 57-60), while the previous JNI functions setDataDir and getLastError were removed entirely. android/app/src/main/java/ai/namastex/forge/MainActivity.kt still declares and invokes external fun setDataDir(...) in onCreate and getLastError() in showServerError, so the native library will throw UnsatisfiedLinkError as soon as those methods are called and the app cannot start the server or surface errors.

Useful? React with 👍 / 👎.

gemini-code-assist bot reviewed Nov 27, 2025

View reviewed changes

jmanhype changed the base branch from main to dev November 27, 2025 17:17

jmanhype force-pushed the claude/fix-android-jni-concurrency-018bhZoHpHSexJLZDoT7u3E2 branch from 854f35f to 45076b6 Compare November 28, 2025 19:10

jmanhype mentioned this pull request Nov 28, 2025

Code Quality: Address minor gemini-code-assist suggestions (7 PRs) #286

Open

chatgpt-codex-connector bot reviewed Nov 28, 2025

View reviewed changes

chatgpt-codex-connector bot reviewed Nov 29, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: resolve unsafe concurrency in Android JNI layer #261

fix: resolve unsafe concurrency in Android JNI layer #261

Uh oh!

jmanhype commented Nov 27, 2025

Uh oh!

gemini-code-assist bot commented Nov 27, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Nov 27, 2025

Uh oh!

namastex888 commented Nov 28, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Nov 28, 2025

Uh oh!

jmanhype commented Nov 29, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Nov 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix: resolve unsafe concurrency in Android JNI layer #261

Are you sure you want to change the base?

fix: resolve unsafe concurrency in Android JNI layer #261

Uh oh!

Conversation

jmanhype commented Nov 27, 2025

Uh oh!

gemini-code-assist bot commented Nov 27, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

namastex888 commented Nov 28, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

jmanhype commented Nov 29, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants