ESM loader: remaining cross-platform decisions and deferred follow-ups from #1965

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

還沒有人認領這個 Issue。

評估

難度
5/5
預估耗時
一週以上
新手友好度
20/100
Issue 類型
重構
描述清晰度
需要釐清
活躍度
冷清
技術堆疊
android, cpp
領域
mobile-dev

研究方向

先將跨平台契約決策與 Android 後續工作分開,然後閱讀 Runtime.h、Runtime::ReadFileText 和 ModuleInternal.cpp,以了解明確列出的原生工作。使用 Android 執行階段測試應用程式和列出的 ns run android LiveSync 檢查作為驗證入口;只有在其中所述的行為已實作或記錄,且相關平台檢查通過後,每個選定項目才算完成。

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

描述

#1965 landed the full loader-overhaul parity with ios#383 plus several hardening rounds. Everything that could be fixed unilaterally was; this issue tracks what deliberately remains: contract questions that need one answer on both platforms, and Android-side follow-ups that were out of the PR's scope. The iOS-side defects the same reviews surfaced are tracked in NativeScript/ios#443 (fixes landed as NativeScript/ios#444).

Cross-platform decisions (one answer, both runtimes)

  • require(esm) facade evaluates under sync-strict (shared bug): createPumpingRequire() of an already-settled TLA module with a default export throws the async-graph refusal naming ns:require-facade:<path>IsGraphAsync() is transitive and the facade requests the TLA target. Violates the contract twice (pumping lifts the TLA refusal; exports-cascade step 3) and the error recommends the API that just failed. Fix shape to agree on: direct Evaluate() + assert-fulfilled for the facade, or a synthetic-module bypass on the evaluation options.
  • Import-map keys carrying queries: Android strips query/fragment before the map consult (query-bearing keys can never match); iOS matches them for static imports. The HTML import-maps spec matches the specifier as written. Pick a side and align.
  • import.meta.url shape: Android exposes the full device path; iOS base-strips to file:///app/.... Portable code matching file:///app/ breaks on Android today. Couples with file:// specifier candidate handling and iOS's dirname inconsistency.
  • Transport identity: Android sends User-Agent: NativeScript-HTTP-ESM and inherits the process CookieHandler; iOS sends no UA and disables cookies. Cookies especially need a joint call.
  • Transport timeouts: Android 15s connect/15s read vs iOS 5s/10s. The docs now state the per-attempt/per-read semantics honestly, but the values should converge (a stalled edge can outlive the graph deadline).
  • HasPendingAsyncModuleGraphWork is process-wide on both platforms: a worker's graph load holds the main boot backstop open. Make it per-isolate.
  • Instantiate-failure registry semantics: iOS evicts the root, Android leaves-and-self-heals; HTML/Node module-map semantics say keep-and-rethrow. Neither does that — pick one.
  • Import attributes are silently ignored (import x from './d.json' with { type: 'json' } loads as JS on both); Node throws. Document or implement, jointly.
  • Import-map validation error-text shape: parse-detail suffix conditionality and multi-offense selection differ slightly between the platforms' JSON parsers. Normalize or document.
  • createRequire's hand-rolled file: parser disagrees with node:url (%2F honored, file://FILE: rejected vs Node); shared. Rebase it on the URL primordial (the comment claiming that's unavailable is wrong on both sides).
  • JSON modules are built textually ("export default " + jsonText), so a "__proto__" key becomes a prototype setter instead of an own property. Shared deviation from JSON-module semantics.
  • require-factory hooks: __requireOverride is Android-only with no producers in-tree; __pauseOnNextRequire is iOS-only (inspector-produced). Converge or delete.
  • The 1s local-entry yield: normative in both docs, but its original purpose was iOS's pre-main() runway. Decide whether to shrink/remove (doc change on both).
  • Waiter-invariant race fixture: the "already-evaluating with no continuation attached" dynamic-import window is documented but unenforced on both platforms — needs a spec that pins it.
  • Pump mechanism shape on iOS (surfaced by porting android#1965's timer specs to iOS): a pumping require started from inside iOS's shared JS-timer CFRunLoopTimer callback can never observe another timer fire — CFRunLoopTimer does not re-enter. Android's pump drains the ordered lane directly and is immune; whether iOS should adopt the same drain shape instead of spinning the runloop is a joint pump-contract call.
  • Frozen-at-mint pumping options: neither suite pins that the options object is validated once and immune to later mutation. Add the spec on both.

Android follow-ups

Reclassified 2026-08-20: the five checked items immediately below turned out to be features present in the merged ios#383 code that the port had missed — implemented in #2021.

  • ESM code cache — landed in #2021: CompileFileEsModule consumes/produces cache blobs (.mcache keying avoids colliding with the classic .cache for the same file).

  • Fetch concurrency cap — landed in #2021: 16 concurrent fetches process-wide (iOS: HTTPMaximumConnectionsPerHost = 16), queued jobs drained by finishing threads, spawn failures delivered as transport errors.

  • Canonical-key transport signatures — landed in #2021: the transport takes keys from callers (iOS's shape); also fixed collapsed-scheme cache-bust marks never matching.

  • HTTP worker entries — landed in #2021: new Worker("http://…") works end-to-end (security gate, vocabulary copy, settle gate, inspector URL), spec included.

  • ns:module binding beside the loader state — landed in #2021 (ios 5ca0c43c), including the ParseImportMap/InstallParsedImportMap split that removes the double parse.

  • Lock the process-global interop caches — done in 7c9c3db3, though not as scoped here. The three caches this item names were already covered by #2013: Console timers and ArgConverter's type-long cache became per-runtime RuntimeState::For<T>(isolate) (the Isolate*-keyed process-global maps are gone entirely), MetadataNode's name/tree-node caches got s_nodeCacheMutex, MetadataReader got its own StateMutex, and JEnv::s_classCache/s_missingClasses plus MethodCache::s_mthod_ctor_signature_cache each got a shared_mutex — all on the lock-the-find-and-the-emplace-but-never-across-the-build discipline this item asks for. Auditing the runtime for what genuinely remained in that class turned up four more, all fixed in 7c9c3db3:

    • File::Buffer — one process-wide 1MB scratch buffer that File::ReadText filled and returned a pointer into. Main and worker runtimes read modules concurrently from their own threads, so one thread's fread overwrote bytes another was still copying out: silent module-source corruption rather than a crash, which is why this one never appeared in a tombstone. It also never saved the allocation it appears to save — every caller goes through the std::string overload, which copied out of it on the next line — so reads now go straight into the returned string and the borrowing overload (no callers) is deleted.
    • JType::EnsureInstance — published *instance before Init had filled clazz/ctor/valueMethodId, so a second thread could find the pointer non-null and call through uninitialised JNI ids, on the hot boxing path. Now std::call_once per Type.
    • CallbackHandlers::Init — runs once per runtime from PrepareV8Runtime, so every worker start rewrote the process-global class and method-id statics (and re-ran MethodCache::Init) while the isolates already running were reading them. Process-global half now under call_once; MetadataNode::Init(isolate) stays per-isolate.
    • MetadataNode::IsJavascriptKeyword — filled a function-local static set<string> behind an empty() check, racing reads from every runtime's thread.

    Audited and deliberately left alone: JsV8InspectorClient::Domains is unguarded but main-isolate only (workers use WorkerInspectorClient, which never registers domain dispatchers); SimpleProfiler::s_frames is racy and stores a pointer into a reallocating vector, but SimpleProfiler::Init has no callers (dead code); PageResource::s_cachedPageResources is write-only, nothing reads it.

  • Runtime::ReadFileText's m_fileWriteMutex is broken three ways (surfaced while removing File::Buffer, which this mutex superficially looked like it guarded — it never did). Its real purpose is LiveSync: NativeScriptSyncServiceSocketImpl.createOrOverrideFile() takes runtime.lock() so the runtime does not compile a half-written hot-synced module. But (1) m_fileWriteMutex is a non-static member (Runtime.h), so LiveSync locking one runtime excludes nothing on a worker's thread — precisely the case that matters; (2) the lock is taken after prepareFile() deletes the file and new FileOutputStream truncates it, so it covers only fos.write and not the window a reader actually hits; (3) the finally calls runtime.unlock() even when lock() was never reached (a throw from mkdirs()/new FileOutputStream), which is UB and on bionic can release a lock the JS thread holds inside ReadFileText. Fix direction: make the write atomic — temp file plus same-directory ATOMIC_MOVE — instead of locking readers. rename(2) within a directory is atomic, so a reader gets the whole old or the whole new file; that also closes the delete/truncate window no reader-side lock can reach, and drops a mutex from every debug module read. m_fileWriteMutex and Runtime::Lock/Unlock then delete cleanly, keeping com.tns.Runtime.lock()/unlock() as deprecated no-ops since they are public and external tooling may call them. Implemented in #2022 — unmerged: LiveSync has no automated coverage in the runtime test app, so it needs a manual hot-sync check (ns run android, edit a file, confirm reload; also a newly-created file and a deleted one, since prepareFile() is gone) before merging.

  • NS_DEBUG is unreachable on a real device (env var); the docs call it "the only way to trace boot". A debug.nativescript.* system property (pattern already used in Runtime::GetAndroidVersion) would make it real.

  • Remaining K14 leftovers: three resolver/import empty-return-without-exception windows in teardown/OOM paths; catch (std::exception e) by value in ModuleInternal.cpp; IsolateData::RUNTIME is never cleared (the TryGetRuntime guards are defense-in-depth only and one teardown comment overstates the invariant).

  • NS_TIMERS_NESTING_CLAMP is documented opt-in and defined by no build file — confirm that's intended (the HTML 4ms nesting clamp is currently off everywhere, including the test suite).

  • Verify @nativescript/core's setTimeout rides __ns__setTimeout: the pumping-require timer story assumes it (the test app's own polyfill is a Java Handler post that no pump can dispatch, by construction). Confirm against core before advertising the behavior.

  • Dev-server smoke test against @nativescript/vite (the same gate as iOS): validates the canonicalization change, the vocabulary purge, the MIME gate, and the ESM-entry routing — and, if the sync-fetch anomaly guard stays cold, authorizes deleting it.

  • Accepted-and-documented, revisit if it bites: the boot backstop blocks the launching thread (up to 120s, dev-time HTTP entries only) by explicit decision — runtime.run() returning entry-settled is load-bearing for NativeScriptActivity; require.resolve/require.cache/require.main remain absent (feature-detectable).

主要語言
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 摘要。