NativeScript / NativeScript/android

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

Đang mở
#2,019 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

Ngôn ngữ chính
C++
Star
563
Fork
144
Merge trung bình
10 giờ 46 phút
Pull request đã merge (30 ngày)
14

Mô tả

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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu với runtime-binding-generator/Dump.java, DexFactory.java và ClassStorageServiceImpl.retrieveClass, sau đó lần theo các đường dẫn ClassResolver và JEnv được nêu trong báo cáo. Xem xét extendClassNameTests.js để tìm các trường hợp xác thực đã bị comment. Công việc được xem là hoàn tất khi việc tạo đồng thời và quyền truy cập của loader an toàn, các artifact chưa hoàn chỉnh không được persist và các lỗi về đặt tên và cache được liệt kê có kiểm thử hồi quy bao phủ.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
android, cpp, java
Lĩnh vực
mobile-dev
Loại issue
Lỗi
Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
42/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.