fix(android): Fix JNI signature mismatch causing SIGSEGV on Android 11+

[0xbrayo]

• Fixed initialize() function to return jstring instead of void to match Java signature
• Changed jstring_to_string() to return Option instead of panicking on error
• Updated all call sites to handle the Option return type safely

This resolves the crash on newer Android versions where JNI signature mismatch was causing segmentation faults during app initialization.

---

Important

Fixes JNI signature mismatch in initialize() and improves error handling in jstring_to_string() to prevent crashes on Android 11+.

• Behavior:
  • initialize() in mod.rs now returns jstring instead of void to match Java signature, fixing crash on Android 11+.
  • jstring_to_string() in mod.rs now returns Option<String> instead of panicking, handling conversion errors gracefully.
• Call Sites:
  • Updated all call sites in mod.rs to handle Option return type from jstring_to_string() safely, preventing potential crashes.

^{This description was created by for 04f5bb7. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 04f5bb7 in 1 minute and 21 seconds. Click for details.

• Reviewed 111 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 5 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. aw-server/src/android/mod.rs:144

• Draft comment:
  Note: The initialize() function’s signature has been updated to return a jstring, which now aligns with the Java side expectations. Consider using a thread-safe mechanism (like Once or an AtomicBool) for managing the INITIALIZED flag to prevent potential race conditions if initialize() is called concurrently.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The first part about jstring return type is just describing what changed, not suggesting any improvements. The second part about thread safety is a valid concern since INITIALIZED is a global mutable static accessed in unsafe blocks. However, this appears to be Android JNI initialization code which typically runs once during app startup, so concurrent access is unlikely in practice. I may be underestimating the thread safety risks. Even if initialization typically happens once, there's no guarantee against concurrent calls. While thread safety is a valid concern, the comment doesn't provide strong evidence that this is actually problematic in the Android JNI context. The initialization pattern shown is fairly common in JNI code. The comment should be deleted as it's partly descriptive of changes rather than suggesting fixes, and the thread safety suggestion, while technically valid, lacks strong evidence that it's a real issue in this context.

2. aw-server/src/android/mod.rs:175

• Draft comment:
  Observation: In setDataDir, the conversion of the input JString is safely handled via match. Optionally, consider logging an error message in the None branch to assist with debugging when the conversion fails.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

3. aw-server/src/android/mod.rs:202

• Draft comment:
  Observation: In createBucket, the use of pattern matching on jstring_to_string improves robustness by avoiding panics on conversion errors. This pattern is consistently applied in the subsequent functions.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

4. aw-server/src/android/mod.rs:227

• Draft comment:
  Observation: Similarly, in the heartbeat function, pattern matching on jstring_to_string for both bucket_id and event ensures that malformed JNI strings are gracefully handled. Again, consider logging these failures for easier troubleshooting.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

5. aw-server/src/android/mod.rs:261

• Draft comment:
  Observation: In getEvents, the pattern match on jstring_to_string is consistent with the other functions, which improves error handling and avoids panics if the JNI string conversion fails.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_j2Ge5B5xpTvubOSb

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Suggestion: In the new implementation of jstring_to_string (lines 85–95), consider using the ? operator to simplify the error handling instead of nested matches. For example, you could write:

let java_str = env.get_string(string).ok()?;
let c_str = CStr::from_ptr(java_str.as_ptr());
return c_str.to_str().ok().map(|s| s.to_owned());

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.76%. Comparing base (656f3c9) to head (ea88658).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #542      +/-   ##
==========================================
- Coverage   70.81%   69.76%   -1.06%     
==========================================
  Files          51       54       +3     
  Lines        2916     3152     +236     
==========================================
+ Hits         2065     2199     +134     
- Misses        851      953     +102     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[petrroll]

Is there no way to catch these (so they match) programatically?

[petrroll]

Wouldn't it make sense to debug log? Error log or smth?

What happens if you can't set the data dir? Does the rest handle that somewhat sanely?

[0xbrayo]

yeah, a debug log makes more sense, here. Will do.

[petrroll]

BTW, did you find out how these can fail?

[0xbrayo]

I only figured out they were failing by using a lot of trace logs. Then updated them didn't investigate much about how exactly they were failing.