[edusperoni]

Description

The crash. Apps intermittently die with a native crash when the main runtime generates a proxy class at runtime, i.e. an extend() the static binding generator didn't see:

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x8
#00 libart.so art::mirror::String::ToModifiedUtf8()+16
#01 libart.so art::ClassLinker::RegisterDexFileLocked(...)+148
#02 libart.so art::ClassLinker::RegisterDexFile(...)
#03 libart.so art::ClassLinker::DefineClass(...)
#04 libart.so art::ClassLinker::FindClassInBaseDexClassLoader(...)
#05 libart.so VMClassLoader_findLoadedClass
    java.lang.ClassLoader.loadClass ← com.tns.DexFactory.resolveClass ← MetadataNode::ExtendMethodCallback

It shows up in our test suite on the API 36 emulator, a few times a day across runs.

Cause: an ART GC bug. ClassLinker::AllocDexCache stores the dex location into the new DexCache, then RegisterDexFile waits in a ScopedGCCriticalSection for an in-flight collection. After that wait, DexCache::location_ reads back as null, while class_loader_ and dex_file_ (stored after the wait) are intact. RegisterDexFileLocked then dereferences the null location. The tombstone heap dumps show this directly, and in some crashes the location String is still on the heap right after the DexCache.

What we found out:

• It reproduces in a plain Android app with no NativeScript code.
• It happens with ART's mark-compact collector (CMC), in generational mode (the default on this image) and, less often, in non-generational mode.
• It needs the dex to be registered with a long-lived class loader, which is what DexFactory does for main-runtime proxies since fix(DexFactory): register injected proxy dex with a single class loader #1968 (addDexPath into the app's PathClassLoader, so Class.forName finds them). With a fresh DexClassLoader per jar it never crashed, but that would lose Class.forName visibility.
• It needs a burst of reference-heavy allocation right before registration. Proxy generation (ASMDEX) is exactly that.
• We're filing it with the ART team separately, with the standalone repro.

The workaround. A blocking Runtime.getRuntime().gc() immediately before the injected dex is registered, so no collection is in flight when RegisterDexFile runs. Results:

Setup	Without	With gc()
Stress harness in the test app, realistic allocation	37/100 runs crashed	0/40
Stress harness, adversarial background allocator, n=800	7/30	0/20
Standalone repro app (no NativeScript), generational CMC	29/40	0/30
Standalone repro app, non-generational CMC	9/40	0/20
Stress harness, adversarial background allocator, n=3000	3/6	1/4

It isn't a guarantee. When another thread allocates non-stop, a background GC queued behind the explicit one can start right after it. That still cut crashes about tenfold (1 crash in ~27k registrations).

Cost. One explicit GC, about 4–14 ms on test-suite-sized heaps and more on large heaps, on the first use of each runtime-generated proxy in a process. Since #2101 the prebuilt classes are found, so runtime generation only happens for extends the generator can't see: rare in production builds, but every extend in HMR dev mode. The full test suite ran in 116 s with the change, against 117–121 s without.

Only where the collector may be mark-compact. ProxyRegistrationGc decides once per process, on the first injection, whether the collection is needed:

• Below API 31: skipped. ART only became updatable through Play system updates in Android 12, so these devices can't be running mark-compact.
• API 31+: skipped only when /proc/self/maps positively identifies the concurrent-copying collector. That means [anon:dalvik-...] mappings containing concurrent copying or (region space), and none containing mark-compact / Mark Compact. Both sets of names come from ART's collector setup; I checked them on API 33 (ART module 331711080) and API 36 (361153460) processes.
• Anything else collects: mark-compact (generational or not), an unrecognized collector, renamed mappings in a future ART, kernels without anonymous VMA names, or an unreadable maps file. Being wrong in that direction costs a few milliseconds; being wrong the other way would mean a crash.

Does your commit message include the wording below to reference a specific issue in this repo?

No issue.

Related Pull Requests

• fix(DexFactory): add support for injecting DEX into parent class loader #1951 / fix(DexFactory): register injected proxy dex with a single class loader #1968 added the injection into the app's PathClassLoader.
• fix: load prebuilt classes for anonymous extends #2101 made prebuilt classes load for anonymous extends, which keeps runtime generation rare.

Does your pull request have unit tests?

Yes, for the guard. testProxyRegistrationGc.js checks the decision on synthetic maps:

• mark-compact collects
• concurrent copying skips
• below API 31 skips
• unnamed mappings collect
• an unknown collector collects
• mixed names collect
• the live check: the decision matches the collector the test process is actually running

The GC race itself needs hundreds of registrations under allocation pressure to show up, so it's covered by the stress numbers above rather than a suite spec.

./gradlew runtestsAndVerifyResults -Pabis=arm64-v8a, full suite:

Emulator	Collector	Specs	Failures	Explicit GCs logged by the test app
API 36 (ART 361153460)	mark-compact	1552	0	134
API 33 (ART 331711080)	concurrent copying	1552	0	20 (the tests' own System.gc() calls)

Summary by CodeRabbit

• Bug Fixes
  • Improved runtime reliability when loading generated classes in supported Android configurations by performing garbage collection before class-loader injection when required.
  • Existing isolated class loading remains available when injection conditions are not met.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

ProxyRegistrationGc detects collector mappings and reports whether collection is required. DexFactory.resolveClass uses that result before eligible parent-loader dex injection. The test app now loads tests for collector detection and mapping behavior.

Changes: Dex class loading

Layer / File(s)	Summary
Collector detection and tests test-app/runtime/src/main/java/com/tns/ProxyRegistrationGc.java, test-app/app/src/main/assets/app/tests/testProxyRegistrationGc.js, test-app/app/src/main/assets/app/mainpage.js	ProxyRegistrationGc checks the SDK level and process mappings. It returns true for mark-compact mappings and for unrecognized mappings, and false for recognized concurrent-copying mappings. Tests cover parser cases and process-level detection.
Parent-loader injection conditions test-app/runtime/src/main/java/com/tns/DexFactory.java	When parent-loader injection is enabled for a BaseDexClassLoader, resolveClass requests garbage collection if ProxyRegistrationGc.isRequired() returns true. The isolated-loader fallback remains unchanged.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant DexFactory
  participant ProxyRegistrationGc
  participant ProcessMaps
  participant Runtime
  DexFactory->>ProxyRegistrationGc: Check whether collection is required
  ProxyRegistrationGc->>ProcessMaps: Read /proc/self/maps
  ProcessMaps-->>ProxyRegistrationGc: Return collector mappings
  ProxyRegistrationGc-->>DexFactory: Return collection requirement
  DexFactory->>Runtime: Request GC before eligible injection

Loading

Suggested reviewers: nathanwalker

Merge Risk: Minimal · up to 1fe90

The change adds a conditional explicit GC before proxy dex registration to reduce native crashes. No concrete merge-blocking issue was found. The author notes the workaround is not a guarantee, and some residual crash risk remains.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: DexFactory now runs a blocking GC before registering injected proxy dex files. The raw summary confirms this GC call in DexFactory.resolveClass, gated by ProxyRegistra…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.

Full details: Title check

Explanation

The title names the main change: DexFactory now runs a blocking GC before registering injected proxy dex files. The raw summary confirms this GC call in DexFactory.resolveClass, gated by ProxyRegistrationGc.isRequired(). The fix(DexFactory) scope is accurate. The title does not mention the collector detection, but the title does not need to cover every detail.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: NativeScript/android
• Branch: fix/dexfactory-gc-before-inject
• File: .coderabbit.yaml
• Commit: 1fe902b
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: abf50e8d-663e-40ad-83be-04e29ce3840a

Commits

Reviewing files that changed from the base of the PR and between 3d60dfa and 1fe902b.

Files selected for processing: 4

• test-app/app/src/main/assets/app/mainpage.js
• test-app/app/src/main/assets/app/tests/testProxyRegistrationGc.js
• test-app/runtime/src/main/java/com/tns/DexFactory.java
• test-app/runtime/src/main/java/com/tns/ProxyRegistrationGc.java

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.