Skip to content

Commit

Permalink
JniWrapperCache: prevent double removal of entry from cache
Browse files Browse the repository at this point in the history
When garbage collector clears the reference and we identify it
in 'JniWrapperCache::cache_wrapper_impl()' we remove the entry
from cache. This ensures, that references are not leaked in
'convert_to_jni()' function.

However, this removal of entry from cache had very bad consequence.
For the already cleared object disposal of cached entry can be
scheduled after new entry for the same pointer is added to cache.
In such situation the disposal scheduled for old object removed
entry related to new object.

In order to prevent that a counter was introduced. The last scheduled
removal from cache erases the entry -- the remaining ones do not do
anything.

Signed-off-by: Patryk Wrobel <[email protected]>
  • Loading branch information
pwrobeldev committed Feb 3, 2025
1 parent 1432716 commit a0278da
Showing 1 changed file with 20 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,13 @@ JniReference<jobject> JniWrapperCache::get_cached_wrapper_impl(JNIEnv* jenv, con
// Object reference has been cleared by garbage collector.
// To avoid leaks we need to delete the weak ref object here.
jenv->DeleteWeakGlobalRef(iter->second.obj);
s_wrapper_cache.erase(iter);
iter->second.obj = nullptr;
// However, because we "remove" the entry from cache here we need to ensure that
// the cleanup, which will be scheduled for already cleared object (which has not
// been performed yet) does not touch any cache entry added later (an entry for new object
// will be added by 'convert_to_jni()').
iter->second.scheduled_removals++;
return {};
} else {
Expand All @@ -95,7 +101,19 @@ void JniWrapperCache::remove_cached_wrapper_impl(JNIEnv* jenv, const void* obj_p
#endif

if (iter != s_wrapper_cache.end()) {
jenv->DeleteWeakGlobalRef(iter->second.obj);
// Firstly, verify if there were multiple cleanups scheduled for the given entry.
// This situation may happen if the exceptional removal of entry happened in
// JniWrapperCache::get_cached_wrapper_impl() when the weak reference was cleared by
// garbage collector. In such a case does not touch the entry - the last scheduled
// cleanup will remove it.
if (iter->second.scheduled_removals > 0) {
iter->second.scheduled_removals--;
return;
}

if (iter->second.obj != nullptr) {
jenv->DeleteWeakGlobalRef(iter->second.obj);
}
s_wrapper_cache.erase(iter);
}
}
Expand Down

0 comments on commit a0278da

Please sign in to comment.