NativeScript / NativeScript/android
Proxy dex generation: thread-safety and latent naming bugs (follow-up to #2016)
Dieses Issue hat noch niemand übernommen.
- Vorherrschende Sprache
- C++
- Sterne
- 563
- Forks
- 144
- Ø Merge
- 10 Std. 46 Min.
- Gemergte PRs (30 T.)
- 14
Beschreibung
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.
Beitragsleitfaden
Erste Schritte
- Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
- Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
- Forke das Repository und arbeite in einem Branch.
- Öffne einen Pull Request, der die Issue-Nummer nennt.
Rechercherichtung
Beginne mit runtime-binding-generator/Dump.java, DexFactory.java und ClassStorageServiceImpl.retrieveClass und verfolge anschließend die im Bericht genannten ClassResolver- und JEnv-Pfade. Überprüfe extendClassNameTests.js auf die auskommentierten Validierungsfälle. Als erledigt gilt die Aufgabe, wenn die nebenläufige Generierung und der Loader-Zugriff sicher sind, unvollständige Artefakte nicht persistiert werden und die aufgeführten Namens- und Cache-Fehler durch Regressionstests abgedeckt sind.
Vom Indexierungsmodell aus dem Issue-Text verfasst.
Bewertung
- Tech-Stack
- android, cpp, java
- Bereich
- mobile-dev
- Issue-Typ
- Bug
- Schwierigkeit
- 5/5
- Geschätzter Aufwand
- Über eine Woche
- Aktivitätsstatus
- Ruhig
- Klarheit
- Größtenteils klar
- Anfängerfreundlichkeit
- 42/100