-
-
Notifications
You must be signed in to change notification settings - Fork 140
fix: prevent crash when jweak points to null #1881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds cache validation functionality to the LRUCache implementation to check for stale weak global references in the ObjectManager. It introduces a callback mechanism to validate cached entries before they are used, preventing the use of invalidated Java weak references.
Key changes:
- Added a validation callback parameter to LRUCache constructor to check if cached entries are still valid
- Implemented validation logic that evicts invalid cache entries before returning them
- Switched from std::unordered_map to robin_hood::unordered_map for improved performance
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test-app/runtime/src/main/cpp/ObjectManager.h | Added declaration for ValidateWeakGlobalRefCallback method |
| test-app/runtime/src/main/cpp/ObjectManager.cpp | Implemented validation callback and updated cache initialization to use it; removed unused variable |
| test-app/runtime/src/main/cpp/LRUCache.h | Added validation callback support, implemented evictKey helper method, and switched to robin_hood hash map |
| test-app/runtime/src/main/cpp/JEnv.h | Added isSameObject helper method for JNI object comparison |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| env.DeleteWeakGlobalRef(object); | ||
| } | ||
|
|
||
| bool ObjectManager::ValidateWeakGlobalRefCallback(const int &javaObjectID, const jweak &object, void *state) { |
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The javaObjectID and state parameters are unused in this function. If they are required by the callback signature but not needed for the validation logic, consider adding a comment explaining why they are unused or use (void) casts to explicitly mark them as intentionally unused to prevent compiler warnings.
| bool ObjectManager::ValidateWeakGlobalRefCallback(const int &javaObjectID, const jweak &object, void *state) { | |
| bool ObjectManager::ValidateWeakGlobalRefCallback(const int &javaObjectID, const jweak &object, void *state) { | |
| (void)javaObjectID; | |
| (void)state; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be a decent suggestion here @edusperoni?
Occasionally (mostly on android API 26.1+) a jweak reference might point to NULL even when the actual Java object exists.
I believe this is a JNI/Android bug, but as a workaround, we now check if this reference is null, and if so we evict the key from the cache which then is going to re-fetch it and error out if it's actually null in Java