NativeScript / NativeScript/android

Proxy dex generation: thread-safety and latent naming bugs (follow-up to #2016)

Offen
#2,019 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

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.methodDescriptorBuilder is a static final StringBuffer used as setLength(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.
  • EACCES on the loser thread: jarFile.exists() / setReadOnly() is a check-then-act pair (DexFactory.java jar assembly), and setReadOnly() 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.
  • ConcurrentModificationException window: ClassStorageServiceImpl.retrieveClass iterates the loaders collection (an unmodifiableCollection over a synchronizedSet) without holding its lock while storeClassaddClassLoader mutates 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 ….dext shapes can) mangles both names identically, so it works until two distinct classes mangle to the same jar. Use a suffix strip.
  • $_ normalization is applied to className but never to baseClassName, so Interface.extend({...}) on a nested interface computes classNameToLoad = com.tns.gen.…$… while the generator emits …_…ClassNotFoundException. Pre-existing; sits on the exact line #2016 guards.
  • The two prefix predicates disagree: ClassResolver tests startsWith("com.tns.gen"), DexFactory tests "com.tns.gen." (trailing dot). A name like com.tns.generated.Foo is a binding class to one and a named proxy to the other.
  • com.tns.tests.* is excluded from isBindingClass, 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 (ValidateExtendArguments is skipped on the hasDot branch), and the extend-name validation specs in extendClassNameTests.js are commented out. A named proxy colliding with a derived anonymous name fails with a bare CNFE.
  • JEnv::InsertClassIntoCache caches nullptr on 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

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Ö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

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.