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

未關閉
#2,019 0 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視

還沒有人認領這個 Issue。

評估

難度
5/5
預估耗時
一週以上
新手友好度
42/100
Issue 類型
缺陷
描述清晰度
基本清楚
活躍度
冷清
技術堆疊
android, cpp, java
領域
mobile-dev

研究方向

從 runtime-binding-generator/Dump.java、DexFactory.java 和 ClassStorageServiceImpl.retrieveClass 開始,接著追蹤報告中提到的 ClassResolver 和 JEnv 路徑。檢查 extendClassNameTests.js 中被註解的驗證案例。完成的標準是並行產生和 loader 存取都是安全的、不會持久化不完整的 artifact,且列出的命名和快取缺陷都有回歸測試涵蓋。

由索引模型根據 Issue 內容生成。

描述

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.

主要語言
C++
星號
563
分支
144
平均合併
10 小時 46 分鐘
30 天內合併 PR
14

貢獻指南

開啟貢獻指南

從這裡開始

  1. 先讀完整個 Issue,再讀專案的貢獻指南。
  2. 在 Issue 下留言說明你要接手 —— 這能避免兩個人做同樣的事。
  3. Fork 儲存庫,在一個分支上完成修改。
  4. 送出 Pull Request,並在描述裡引用這個 Issue 編號。

NativeScript/android 的其他 Issue

查看 NativeScript/android 的全部 Issue

相似的 Issue

更多 C++ Issue

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。