NativeScript / NativeScript/android
Proxy dex generation: thread-safety and latent naming bugs (follow-up to #2016)
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 563
- Forks
- 144
- Avg merge
- 10h 46m
- Merged PRs (30d)
- 14
Description
Follow-up to #2016, which makes runtime proxy dex generation a routine path (dev servers keeping @nativescript/core off disk) instead of a rare one. None of the items below block that PR — they are pre-existing defects in the generation path whose exposure it raises, plus small latent bugs found while reviewing it. Intended to be picked up after the ESM/loader work lands.
Concurrency (the substantive part)
Proxy generation has no synchronization, but it is reachable from every runtime's thread (extend works on workers; each Runtime has its own DexFactory, but they share one dexDir and the static state below):
- Silent dex corruption:
Dump.methodDescriptorBuilderis astatic final StringBufferused assetLength(0)→ append →toString()(runtime-binding-generator,Dump.java:27). Two concurrent generations interleave and bake wrong method descriptors into a dex — no error, just a wrong class.Dump.interfaceImplementedInterfaces[1] = classSignature(Dump.java:810,820) is the same hazard on a static array. EACCESon the loser thread:jarFile.exists()/setReadOnly()is a check-then-act pair (DexFactory.javajar assembly), andsetReadOnly()runs on every resolve including cache hits, widening the window. Two threads resolving the same class can leave one opening a 0444 file for write.- Truncated jar persisted read-only:
fi.read(dexData, 0, dexData.length)is a single unchecked read (DexFactory.java, jar assembly). A short read — e.g. racing a concurrent write of the same dex — zero-pads the jar, which is then made read-only and reused on subsequent launches within the install. ConcurrentModificationExceptionwindow:ClassStorageServiceImpl.retrieveClassiterates the loaders collection (anunmodifiableCollectionover asynchronizedSet) without holding its lock whilestoreClass→addClassLoadermutates it. Every runtime-generated proxy adds a loader, so #2016 directly raises the hit rate (and makes the miss path O(loaders)).
Suggested shape: make Dump's scratch state instance-local (it already is instantiated per ProxyGenerator); loop the read or use Files.readAllBytes; write the jar to a temp name and atomically rename; synchronize the loaders iteration on the underlying set.
Latent bugs / nits
dexFile.getPath().replace(".dex", ".jar")replaces all occurrences, not the suffix — a package segment containing.dex(e.g.com.example.dexter…does not, but….dextshapes can) mangles both names identically, so it works until two distinct classes mangle to the same jar. Use a suffix strip.$→_normalization is applied toclassNamebut never tobaseClassName, soInterface.extend({...})on a nested interface computesclassNameToLoad = com.tns.gen.…$…while the generator emits…_…→ClassNotFoundException. Pre-existing; sits on the exact line #2016 guards.- The two prefix predicates disagree:
ClassResolvertestsstartsWith("com.tns.gen"),DexFactorytests"com.tns.gen."(trailing dot). A name likecom.tns.generated.Foois a binding class to one and a named proxy to the other. com.tns.tests.*is excluded fromisBindingClass, so a missing test class now falls through to runtime generation instead of throwing — an unintended widening from #2016's fallthrough.- There is no name validation at all for dotted extend names (
ValidateExtendArgumentsis skipped on thehasDotbranch), and the extend-name validation specs inextendClassNameTests.jsare commented out. A named proxy colliding with a derived anonymous name fails with a bare CNFE. JEnv::InsertClassIntoCachecachesnullptron a failed resolve, and the cache read treats that as a miss forever — a name that fails once and succeeds later re-crosses JNI on every lookup (perf only).
Explicitly not included
A "migration sweep" for legacy un-thumbed cache files was considered and rejected: dexDir lives under the app's code_cache, which the platform wipes on every app upgrade — the same event that changes the thumb — so pre-#2016 files cannot survive into a post-#2016 install. The only residue is the rare fallback dir (files/secondary-dexes, used when code_cache is unusable), which is not platform-wiped; not worth machinery.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with runtime-binding-generator/Dump.java, DexFactory.java, and ClassStorageServiceImpl.retrieveClass, then trace the ClassResolver and JEnv paths named in the report. Review extendClassNameTests.js for the commented validation cases. Done means concurrent generation and loader access are safe, incomplete artifacts are not persisted, and the listed naming and cache defects have regression coverage.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- android, cpp, java
- Domain
- mobile-dev
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100