[jni] Add bounded LRU cache for jclass references

[sagar-h007]

• This PR adds a thread-safe LRU cache for jclass global references, with promotion on access and proper cleanup on eviction. The Dart API exposes the cache and allows configuring its capacity.

• On the code generation side, jnigen now generates lazy getters that resolve classes through the cache instead of using static final fields, so all generated bindings share the same caching behavior.

• Tests were added to cover basic cache usage and eviction, and native builds and analysis were verified locally.

• Fixes: Cache JClasses in a map instead of using static final #1677.

I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

• See our contributor guide for general expectations for PRs.
• Larger or significant changes should be discussed in an issue before creating a PR.
• Contributions to our repos should follow the Dart style guide and use dart format.
• Most changes should add an entry to the changelog and may need to rev the pubspec package version.
• Changes to packages require corresponding tests.

Many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

Note: The Dart team is trialing Gemini Code Assist. Don't take its comments as final Dart team feedback. Use the suggestions if they're helpful; otherwise, wait for a human reviewer.

[liamappelbe]

I think it'd be better (simpler, easier to maintain) to just put the LRU cache in Dart. This will mean that the cache will be per-isolate, but that's fine.

[sagar-h007]

I think it'd be better (simpler, easier to maintain) to just put the LRU cache in Dart. This will mean that the cache will be per-isolate, but that's fine.

@liamappelbe Thanks for the feedback, that makes sense. I agree that a Dart-side LRU would be simpler to maintain overall. My original motivation for putting it on the native side was to centralize management of JNI global references and keep native memory usage bounded, but I can see the appeal of keeping this logic in Dart, even if that means the cache is per-isolate.

If that’s the preferred direction, I’m happy to rework this to move the LRU into Dart and keep the native layer minimal, while still avoiding static cached globals and unbounded JNI refs.

[github-actions]

PR Health

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
jni	Breaking	0.15.2	0.16.0-wip	0.16.0-wip	✔️

This check can be disabled by tagging the PR with skip-breaking-check.

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

API leaks ⚠️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package	Leaked API symbol	Leaking sources
jni	$JIterator	core_bindings.dart::JIterator::implementIn::$impl core_bindings.dart::JIterator::implement::$impl
jni	$JCollection	core_bindings.dart::JCollection::implementIn::$impl core_bindings.dart::JCollection::implement::$impl
jni	$JList	core_bindings.dart::JList::implementIn::$impl core_bindings.dart::JList::implement::$impl
jni	$JMap$JEntry	core_bindings.dart::JMap$JEntry::implementIn::$impl core_bindings.dart::JMap$JEntry::implement::$impl
jni	$JSet	core_bindings.dart::JSet::implementIn::$impl core_bindings.dart::JSet::implement::$impl
jni	$JMap	core_bindings.dart::JMap::implementIn::$impl core_bindings.dart::JMap::implement::$impl

This check can be disabled by tagging the PR with skip-leaking-check.

[liamappelbe]

I'm confused. You've requested another review, and the commit says you fixed the ownership model, but there are no changes other than documentation. JClass is still creating more JGlobalReferences each time its constructed, defeating the purpose of the cache.

[sagar-h007]

@liamappelbe sorry for the confusion — you’re right that the previous commit mostly clarified documentation and didn’t actually change the behavior, so JClass was still creating additional JGlobalReferences.
I’ve now reworked the cache to change the actual ownership model, not just document it. The cache is now the sole owner of each JGlobalReference, and wrappers borrow from a shared cache entry instead of creating new global refs.

[sagar-h007]

@liamappelbe I’ve reworked the cache to use a refcounted, borrowed-ownership model:
•The cache now owns exactly one JGlobalReference per class.
•Callers/wrappers borrow that shared reference instead of creating new global refs.
•Each cache entry tracks a borrowCount and whether it’s been evicted.
•When an entry is evicted, it’s removed from the cache right away, but the underlying JGlobalReference is only deleted once the last borrower releases it.
•This keeps the total number of JGlobalReferences bounded to the number of cached classes, while still making sure wrapper finalizers can’t invalidate cache entries.

As a result, JClass.forNameCached no longer creates new global refs on each call. The cache is now fully responsible for managing the lifetime of the single global ref per class.

[liamappelbe]

I guess this ref counting approach would work, but it's very complicated. The solution I was thinking of was just to store the JClass directly in the cache, and change the constructor to a factory constructor that gets the JClass instance from the cache. The cache itself would be responsible for constructing the JClass.

[sagar-h007]

@liamappelbe Thanks,I agree the refcounted approach adds quite a bit of complexity, even if it solves the ownership issue.

Storing JClass directly in the cache and making the cache responsible for constructing and owning the JClass instances sounds a lot simpler and easier to reason about. That would also naturally avoid creating extra JGlobalReferences and sidestep the wrapper finalizer problems.

I’ll rework this to move toward that model: have the cache own and return JClass instances via a factory-style API, and remove the refcounted borrowed-reference machinery. I’ll update the PR accordingly.

[sagar-h007]

@liamappelbe , Is there anything that needs to be changed?

[liamappelbe]

Oops. I've had a lot of PRs to review recently and this one dropped off my radar. There are a few comments that you've marked resolved, but aren't actually resolved.

1. I think you still need to regenerate jni_bindings_generated.dart, since all the changes are in Dart-land now.
2. You deleted the documentation on StringMethodsForJni.toNativeChars. Please revert this.

There's also still formatting and analysis errors, so the bots aren't running the tests.

[liamappelbe]

You've changed the code gen, so you'll need to regen all the bindings.

[liamappelbe]

Change this to:
Do NOT hold a long term reference the returned JClass. The cache manages their lifecycle and may evict it at any time.

[liamappelbe]

Looks pretty good. You'll just need to fix the failing tests now. This will need a changelog entry in both jni and jnigen. There's also some formatting issues to fix. See the bots for details. Also, you still need to revert the unnecessary changes to pkgs/jni/lib/src/third_party/jni_bindings_generated.dart

btw I'm going on vacation for a week, so no more code reviews until next week.

[liamappelbe]

Looking good. You'll need to fix the merge conflicts, but it looks like most of those files are generated code, so you can just regenerate the bindings.

[liamappelbe]

Remember to format and analyze your code before you upload it. Looks like you've got some merge issues causing analysis errors. It's better to just regenerate generated code, rather than trying to manually merge it.

[liamappelbe]

More PRs landed in the week since your last sync. If you sync to head again, regen the code, and fix any formatting/analysis errors before you upload, then this won't be a problem.