NativeScript / NativeScript/android
Proxy dex generation: thread-safety and latent naming bugs (follow-up to #2016)
Personne n'a encore pris cette issue.
- Langage dominant
- C++
- Étoiles
- 563
- Forks
- 144
- Merge moyen
- 10 h 46 min
- PR mergées (30 j)
- 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.
Guide de contribution
Ouvrir le guide de contribution
Par où commencer
- Lisez l'issue en entier, puis le guide de contribution du projet.
- Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
- Forkez le dépôt et travaillez sur une branche.
- Ouvrez une pull request qui référence le numéro de l'issue.
Piste de recherche
Commencez par runtime-binding-generator/Dump.java, DexFactory.java et ClassStorageServiceImpl.retrieveClass, puis suivez les chemins ClassResolver et JEnv indiqués dans le rapport. Examinez extendClassNameTests.js pour les cas de validation commentés. Le travail est terminé lorsque la génération concurrente et l’accès du loader sont sûrs, que les artefacts incomplets ne sont pas persistés et que les défauts de nommage et de cache listés sont couverts par des tests de régression.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Évaluation
- Stack technique
- android, cpp, java
- Domaine
- mobile-dev
- Type d'issue
- Bug
- Difficulté
- 5/5
- Temps estimé
- Plus d'une semaine
- Activité
- Calme
- Clarté
- Plutôt claire
- Accessibilité débutants
- 42/100