Repository navigation
fix(ffi/objc): map metadata classes from frameworks loaded after init - #114
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.
Metadata classes whose image is not loaded when the class globals are registered (e.g. ScreenCaptureKit's SCWindow in a plain Node process) never entered mdClassesByPointer, so their instances were wrapped as NSObject. Keep those classes in a pending list and bump a process-wide generation counter from an objc_addLoadImageFunc callback. On a mdClassesByPointer miss, the pending list is re-resolved on the JS thread only if an image has loaded since the last pass, so hits stay a single map lookup and misses add one atomic load.
|
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 5), reworked to keep lookups off the hot path as requested in review.
Summary
When class globals are registered,
objc_getClassreturns nil for classes in frameworks that aren't loaded yet (e.g. ScreenCaptureKit'sSCWindowin a plain Node process). Those classes never enteredmdClassesByPointer, so anSCWindowinstance wrapped asNSObjectwith every propertyundefined.mdClassesByPointer.find(cls).__NSCFStringat each superclass level) cost an empty check plus one atomic load and compare. No string allocation and no name lookup, unlike fix(ffi/objc): forward off-thread method and function callbacks to the JS thread #103's version.SCWindows.objc_addLoadImageFuncis available from macOS 10.15 / iOS 13, at or below the deployment targets.Evidence
Node v26.10.0, plain Node-API build, macOS 26.6.2 (arm64):
SCShareableContentwindows,SCWindowtouched only viawindows.objectAtIndex(0)NSObject,windowID/title/owningApplicationallundefinedSCWindow, realwindowIDandowningApplicationobjc.importafter init;valueForKey("defaultConfiguration")on the shared picker (untypedid)NSObject, propertiesundefinedSCContentSharingPickerConfiguration, real property valuesMACOS_TEST_FFI_BACKEND=napi npm run test:macos: tests=716, failures=0, errors=0, skipped=17npm run test:macos: tests=716, failures=0, errors=0, skipped=10Merge Danger
Door: two-way
Blast Radius: object wrapping in the Objective-C napi bridge. Instances of classes from frameworks loaded after init now wrap as their real class instead of a superclass.