Fix/0.0.95 crashes#383
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds libvndksupport.so to the Android Manifest, implements a workaround to skip GPU initialization on Android 16 and Pixel 10 devices to prevent crashes, and adds a call to cancelProcess() before closing conversations to avoid a use-after-free SIGSEGV. The review feedback identifies critical race conditions and concurrency issues during conversation teardown, recommending the introduction of a Mutex to synchronize lifecycle operations and prevent native crashes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| import android.util.Log | ||
| import android.app.ActivityManager | ||
| import android.content.Context | ||
| import android.os.Build |
There was a problem hiding this comment.
To prevent concurrent access and race conditions on the conversation and engine lifecycles, we should introduce a Mutex to synchronize these operations.
| import android.os.Build | |
| import android.os.Build | |
| import kotlinx.coroutines.sync.Mutex | |
| import kotlinx.coroutines.sync.withLock | |
| private val lifecycleMutex = Mutex() |
There was a problem hiding this comment.
Fixed differently — stopGeneration no longer calls closeConversationSafely at all. It now only signals stop synchronously: currentJob?.cancel() + conversation?.cancelProcess(). This eliminates the close/close race architecturally since only resetConversation and unloadModel ever call close(), and those are serialized via currentJob?.join() in closeConversationSafely.
The remaining concurrent-close case (two unloadModel calls racing) is handled by @Volatile on conversation plus a null-first swap: val conv = conversation ?: return; conversation = null. Only one caller gets the non-null handle and proceeds to close().
A Mutex here would also need to guard the conversation = eng.createConversation() assignment in resetConversation to be correct — otherwise a concurrent caller can still acquire the lock after close and later close a freshly created conversation.
| // Signal the native inference thread to stop before freeing the Conversation object. | ||
| // Without this, the native thread can fire onDone after close() frees the native handle, | ||
| // causing a use-after-free SIGSEGV in nativeSendMessageAsync. | ||
| try { | ||
| conversation?.cancelProcess() | ||
| } catch (e: Exception) { | ||
| Log.w(TAG, "closeConversationSafely — cancelProcess error: ${e.message}") | ||
| } | ||
|
|
||
| try { | ||
| conversation?.close() | ||
| Log.d(TAG, "closeConversationSafely — closed") |
There was a problem hiding this comment.
Critical Race Condition & Native Crash Hazard
Since closeConversationSafely is called from multiple coroutines running concurrently on Dispatchers.Default (e.g., during rapid user interaction, model switching, or stopping generation), there is a critical race condition:
- Concurrent Close / Use-After-Free: Multiple threads can call
close()orcancelProcess()on the same nativeConversationhandle concurrently, leading to native SIGSEGV crashes. - State Corruption: If
resetConversationruns concurrently withcloseConversationSafely, a newly created conversation can be overwritten withnullin thefinallyblock of a concurrentcloseConversationSafelycall, causing subsequent generation attempts to fail withLITERT_NO_CONV.
We should synchronize the conversation teardown using the lifecycleMutex and implement a double-checked locking pattern to ensure thread safety.
// Signal the native inference thread to stop before freeing the Conversation object.
// Without this, the native thread can fire onDone after close() frees the native handle,
// causing a use-after-free SIGSEGV in nativeSendMessageAsync.
try {
lifecycleMutex.withLock {
val conv = conversation
if (conv != null) {
try {
conv.cancelProcess()
} catch (e: Exception) {
Log.w(TAG, "closeConversationSafely — cancelProcess error: ${e.message}")
}
conv.close()
Log.d(TAG, "closeConversationSafely — closed")
}
}
- Add libvndksupport.so to AndroidManifest to fix GPU init on Qualcomm SoCs - Skip GPU backend on Pixel 10 where LiteRT GPU init crashes with uncatchable native abort (libPVROCL OOM/SIGSEGV) - Call cancelProcess() before close() in closeConversationSafely to prevent use-after-free SIGSEGV in nativeSendMessageAsync Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Add @volatile to conversation field so null-first swap in closeConversationSafely is visible across threads; stopGeneration now signals stop synchronously without tearing down the conversation, eliminating the race with unloadModel that caused SIGSEGV Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
Co-Authored-By: Dishit Karia hanmadishit74@gmail.com
dishit-wednesday
force-pushed
the
fix/0.0.95-crashes
branch
from
c446324 to
6745ea1
Compare
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #383 +/- ##
==========================================
- Coverage 81.82% 81.82% -0.01%
==========================================
Files 241 241
Lines 12834 12835 +1
Branches 3534 3535 +1
==========================================
Hits 10502 10502
Misses 1403 1403
- Partials 929 930 +1
🚀 New features to boost your workflow:
|
…GSEGV finishRealtimeTranscribeJob dereferenced the result of job_get() without a null check; when the job was already removed (double-finish on the 30s buffer-full path, or an abort/transcribe race) this caused a SIGSEGV in production (0.0.95). Add a null guard in jni.cpp and make the Java finishRealtimeTranscribe a one-shot (synchronized + isRealtimeFinished) so it cannot fire twice for the same job. Co-Authored-By: Dishit Karia <hanmadishit74@gmail.com>
bring in one line with i button and change colour to yellow
|
1 participant
Summary
Type of Change
Screenshots / Screen Recordings
Android
iOS
Checklist
General
Testing
npm test)React Native Specific
project.pbxproj)SPACING/TYPOGRAPHYconstants from the themeuseThemedStylespattern (not inline or staticStyleSheet.create)FlatList/FlashList(not.map()insideScrollView)Performance & Models
/vs\\)Security
Related Issues
Additional Notes