yglukhov / jnim

Nim - Java bridge
MIT License
184 stars 13 forks source link

[RFC] Ownership model should be reconsidered #29

Closed yglukhov closed 7 years ago

yglukhov commented 7 years ago

Short description: Nim wrappers for jobject/jclass refs should not take ownership of the ref, but rather create a NewGlobalRef to dispose it later in finalizer.

Long description: currently Nim wrappers make some faulty assumptions about the refs they hold:

  1. Refs are local. That might be wrong, when explicitly creating a wrapper with fromJObject.
  2. Local refs are unique, and the wrapper is the only owner of the ref. This may be wrong, because depending on JNI platform, such methods as FindClass, GetObjectArrayElement, etc, may return the same local ref across multiple calls.
  3. Refs are valid when finalizer is called. This is obviously wrong, because local refs may be invalidated as soon as we return from native to java. While Nim finalizer might be called some time after that.

The way to fix: Nim wrappers must never take ownership of the ref. Any call to newJVMObject or fromJObject should be revisited to dispose local refs manually. The wrapper should create a personal unique global ref for itself.

Performance considerations: such changes may potentially hit performance, because currently generated glue code makes heavy use of nim wrappers. In the long run the glue codegen has to be optimized to not use nim wrappers, and only create them whenever needed for the higher-level interface.

Good news: I've already started doing everything above, but I'd want to hear your opinion =) Teaser: I've implemented a way to subclass Java interfaces in Nim, but can't commit it because its unstable due to subj.

vegansk commented 7 years ago

I agree 100%. Thanks!