NativeScript / NativeScript/android
Proxy dex generation: thread-safety and latent naming bugs (follow-up to #2016)
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.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.
Hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- 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.
- Fork repository và làm thay đổi trên một nhánh.
- 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