Repository navigation
feat(ffi/objc): add __releaseNativeCounterpart to the Node-API bridge - #113
Open
NathanWalker wants to merge 481 commits into
Open
NathanWalker wants to merge 481 commits into
NathanWalker wants to merge 481 commits into
Conversation
When a java object is finally freed in Java, we notify the JS side to free the JS counterpart. At this time, the Java object is held weakly in Java too using a WeakReference. While the Java object is weakly held, the WeakReference is strongly kept in a HashMap never getting removed. In this PR we release the WeakReferences too on GC event ensuring that all memory is reclaimed.
…-array args can't be type-checked
Expo installs global.expo as a non-writable property, so exposing the expo.* Java package under that name threw from CreateTopLevelNamespaces on Hermes. Fall back to the $-prefixed name already used for packages that collide with JS keywords.
The react-native target builds the Android tools jars, which compile against jars stored in Git LFS under platforms/android/test-app/**/libs. Without lfs on the Apple job's checkout, those resolve to pointer files and compileKotlin fails with "zip END header not found".
…when it matches its last one Fabric keeps _props across prepareForRecycle while the container is cleared, so diffing against _props skipped setNativeViewHandle when a recycled host was mounted with the handle it last showed, leaving it empty. Diff against the oldProps argument instead, which is null on a fresh mount.
fix(react-native/ios): attach a recycled NativeScriptUIView's handle when it matches its last one
The property-getter, round-trip and expando lookup caches identified a bridge by its address. After a JS reload a new bridge can be allocated where a destroyed one lived and match its entries: the getter cache then returned a raw pointer into the freed bridge (EXC_BAD_ACCESS in preparedObjCInvocationIsInit on the next launch), and cached misses could hide values that exist.
…act-native.json (#85) The config plugin and nativescript-rn configure replaced the whole reactNative.metadata block, so every prebuild or configure run deleted a hand-written metadata.android.include. That list is what keeps classes, including autolinked module packages, in filtered Android release metadata. Merge into the existing metadata instead, accept metadata.android.include in the plugin options, and write the file on Android-only prebuilds too.
* fix(react-native): explain a missing native module at import Keep failing at import when NativeScriptNativeApi is not in the app binary, but with an error that says the package cannot run in Expo Go and that a build made before the package was added must be rebuilt, instead of React Native's generic TurboModuleRegistry invariant. Drop isAvailable() and the Android-specific init() path.
* fix(android): use $name when a Java package's global name is taken Check whether the global already has the package's name and, if it does, expose the package as `$name`, logging a warning. Both the jsi and napi trees now do the same check, so an existing global is never overwritten and no engine-specific read-only handling is needed.
With globals enabled, every key of every enum object became a global, including short member names such as Center, Fill and CloudKit's Zone. Libraries that feature-detect a global by name were misled: Angular's zoneless scheduler checks typeof Zone and crashed at bootstrap. The metadata only stored the shortened member names, so the bridge could not tell which alias was the real C constant. Metadata format v2 records, per enum, the prefix and suffix the generator stripped from its members, and flags members whose stored name is already the full C name. The reader rebuilds every member's exact C name from that; v1 metadata still loads but yields no C names. A shared alias builder (ffi/objc/shared/EnumMemberAliases.h) tags the C name among each member's aliases and is used by both the JSI bridge and the Node-API tree. The bridge exposes getEnumGlobalNames(enumName), and the shared global installer and installGlobals() in @nativescript/react-native define only those names. The Node-API tree defines only C names as globals. Short names and other conveniences stay on the enum object, which also gains the C name as a key, so constants like MKMapTypeStandard and CKErrorZoneBusy now resolve.
* refactor(android): detach through the worker teardown path Narrows the reload fix to what React Native needs: the module tracks its own install, invalidate() detaches, and detaching reuses the teardown a worker already goes through. - Runtime.detach() is detachRuntime() (formerly detachWorkerRuntime) plus Runtime::DisposeRuntime (formerly DisposeWorkerRuntime), now built in the host-runtime flavour too. The separate Runtime::Detach is gone. - ~Runtime still resets the main-runtime statics, and BuildMetadata still builds the tree once. Dropped, to land separately or not at all: throwing on a failed post to the JS thread, the GcListener delivery lock and the notifyGc change, the orphaned-object map, the LRU cache clear, the global-ref delete in ~Runtime, MethodCallbackData ownership, and their napi mirrors. * refactor(android): name napi's runtime teardown DisposeRuntime too
A GC batch could reach a worker runtime while it was being disposed: the GC thread took the runtime from the subscriber map, the worker thread unsubscribed and disposed its native Runtime, and the GC thread then entered notifyGc for it and crashed (SIGSEGV on the GC thread). Each subscriber now carries its own lock. A batch is delivered while holding that lock, and only if the subscriber is still subscribed; unsubscribe() removes the subscriber, then takes its lock to mark it unsubscribed. Once unsubscribe() returns, no GC thread is inside that runtime's notifyGc and none will enter it. The memory monitor's notifyGc() takes the same per-runtime lock instead of the listener-wide one, and unsubscribe() no longer takes the listener-wide lock, so subscribing or unsubscribing one runtime never waits on a GC delivery into another.
… dispose (#92) Disposing a runtime (a worker, or a React Native guest runtime) left three things behind: - ObjectManager's LRU cache of Java objects: ~LRUCache drops its map without calling the evict callback, so every cached weak global ref stayed in the process-wide weak reference table. Add LRUCache::clear(), which evicts each entry, and call it from ObjectManager's dispose. - The JNI global ref ~Runtime holds on the Java com.tns.Runtime. - MethodCallbackData for static methods, extension-only methods and methods added from a skipped base class. CtorFuncCache only owns the instance method data, so these were never deleted (about 180 per worker that touches a handful of classes). MetadataNodeCache now owns them and deletes them in onDisposeEnv/onDisposeRuntime. Applied to both the napi and jsi trees. Split out of #89.
…destroying (#95) DestroyRuntime set is_destroying before flushing the finalizer queue. HostObjectProxyPostFinalizer treats is_destroying as "leave the target handle", a rule carried over from napi, where js_free_napi_env frees leftover refs. engine:: handles have no such owner, so every queued proxy cleanup leaked its JS wrapper, and on QuickJS JS_FreeRuntime aborted on list_empty(&rt->gc_obj_list). The queue is normally drained on the next looper tick. A worker that calls close() from onmessage quits its looper first, which drops that tick, so any proxy collected during the handler was still queued at dispose. terminate() from the parent arrives after the tick has run. Flush the queue first, while the runtime is not yet marked destroying. Fixes #94
s_type_long_operations_cache is process-wide, but each runtime inserts its entry on init and erases it on dispose from its own thread. Make it a tns::ConcurrentMap, as MetadataNode's per-runtime cache already is, in both the napi and jsi trees. Fixes #93
…bal (#83) Without strictGlobal the Worklets plugin captures every name a worklet does not declare into its closure, read on the JS thread when the module is evaluated, where NativeScript's classes do not exist: a module-level worklet naming UILabel failed at startup with "ReferenceError: Property 'UILabel' doesn't exist". The config plugin and configure CLI now write ['react-native-worklets/plugin', { strictGlobal: true }] and ['babel-preset-expo', { worklets: false }] (so the preset adds no second copy), upgrade bare entries in an existing config, and warn when an entry with options lacks strictGlobal. The Babel setup also runs on Android-only prebuilds.
Expose the global __releaseNativeCounterpart(obj) used by the Android runtimes so plugins can release a native object eagerly with the same call on both platforms. It is registered from nativescript_init, so both the engine builds using the napi FFI backend and the plain Node-API runtime get it. Behaviour: - exactly one argument, otherwise throws 'Actual arguments count: "N". Expected: "1".' - anything that is not a live native object wrapper throws '<value> is an object which is not a native wrapper.' - releasing an already released wrapper is a no-op - returns undefined The wrapper is detached from the bridge caches, its wrap (and the wrap of the array proxy target) and native pointer are removed so later use throws instead of reaching freed memory, and the bridge's retain is dropped. The unwrapped pointer is only treated as an id once objectRefs maps it to this exact wrapper, so pointers, references and class constructors are rejected without being dereferenced.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
|
ammarahm-ed
force-pushed
the
android-react-native
branch
2 times, most recently
from
November 8, 2026 12:59
4f87f05 to
9633aba
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #103 (part 4), renamed and aligned with Android as requested in review.
Summary
Adds the global
__releaseNativeCounterpart(obj)to the Objective-C Node-API bridge, matching Android's name and behavior, so plugins can call one thing on both platforms. It replaces #103'sinterop.releaseNativeObject, so there's only one public name.undefined, like Android.ApiTests.jscases (from the old iOS runtime) expect. Android uses different wording for the same cases.nativescript_init, so both the plain Node-API runtime and engine builds that use the napi bridge get it. Typings inpackages/objc-node-api/index.d.ts.Evidence
MACOS_TEST_FFI_BACKEND=napi npm run test:macos: tests=719, failures=0, errors=0, skipped=12. All 8__releaseNativeCounterparttests run and pass (5 existing, now no longerpending, plus 3 new: use after release throws, a second release is a no-op, a JS-defined instance held by anNSMutableArraykeeps working).npm run test:macos(V8 bridge): tests=719, failures=0, errors=0, skipped=13. The V8 bridge doesn't define the global yet, so the release tests staypendingthere.NSMutableArray/NSObjectthrows, every invalid input throws, and 2000 allocate/release cycles followed bygc()finish without a crash.Merge Danger
Door: one-way (new public API)
Blast Radius: Objective-C napi bridge, new global only. Existing code paths are unchanged.