Repository navigation
Loader hardening follow-ups surfaced by the Android parity port (android#1965) #443
Description
The Android parity port of the loader overhaul (NativeScript/android#1965, mirroring #383) included several adversarial review passes that compared both implementations function-by-function. A number of defects and cleanups surfaced on the iOS side. Android has already fixed its analogues where they were shared; this issue catalogues what remains to fix here. Line anchors are against main at ee51d741/3a6332a7.
Bugs
- Worker entry rejection is swallowed.
Worker.mm:264attaches the same handler as both fulfill and reject (Then(onSettled, onSettled)), which marks a TLA entry's rejection handled — it never reaches the worker'sonerrornor the parent's Worker error event. Android's fix routes it through the web's order (worker scopeonerrorfirst, then the parent). Most serious item on this list. - Five
.Check()sites inImportModuleDynamicallyCallbackcan abort the process from user code.ModuleInternalCallbacks.mm:3133-3134, 3151, 3284-3287, 3291, 3296-3299. The bareResolve(...GetModuleNamespace()).Check()at:3291is reachable by a namespace exporting a TDZ-ablethen(import cycle):ResolvereturnsNothing→ CHECK-abort. The preceding guard only probesdefault, neverthen. Android converted these toFromMaybe/reject-with-thrown-reason. - No pump honors
IsExecutionTerminating. All three pump loops (NativeScript.mm:68-85,ModuleInternalCallbacks.mm:1671-1683,ModuleInternal.mm:1400-1425) spin to their deadline afterworker.terminate(), then throw from a terminating isolate. -
deadlineSeconds: NaN/Infinityaccepted natively → infinite pump (ModuleInternal.mm:346-348). The JS mint-time validation coversns:module, but the native layer re-validates nothing. - Dynamic-import failure tail leaks a pending exception and rejects with a bare message (
mm:2281, 2381-2383—"HTTP fetch/compile failed"with no URL, no cause). Android routes the classifier'sfailureReason. - Import map is applied twice on dynamic import, and the second pass drops scopes (
mm:2473-2492, thenmm:3088with an empty referrer). Android resolves once through the shared seam. - Sync transport clears a pending cache-bust mark on any 2xx before classification (
HttpLoader.mm:744) — a200 text/htmlSPA fallback consumes the eviction nonce. The async path (and the contract) clear only on an ok-classified response. - CJS cache key strips extensions:
require('./config.json')afterrequire('./config.js')returns the JS module's exports (mm:673-681). - Static
import './x.js?v=1'from a local referrer throws — the resolver probes a file literally namedx.js?v=1(mm:2443-2452handles this for dynamic import only). Android strips query/fragment in the shared resolution seam. - Registry-key replacement leaks require-facades and identity-index entries (
mm:463, 1540, 2106, 2252): eviction unhooks them, replacement does not — one leaked facade + two Globals per HMR reload of a required module. Android hooks the drop into its shared unindex helper so replacement and eviction behave identically; adopt the same shape. -
ThrowExceptionwith a possibly-empty handle (mm:1280-1281). - Failed Node-API addon
requiresilently returnsundefined(mm:500-507). - Require failures are re-wrapped, destroying the original error's identity/class (
mm:622-667). Android rethrows unchanged. - Worker
.mjsentries get 6 prepended lines (mm:1694-1713): stack line numbers shift by 6, and an entry declaring its ownconst selfbecomes a SyntaxError.
Principle / parity cleanups
- Webpack client vocabulary residue: the
__webpack_ids__probe and a hardcodedfile:///app/runtime.mjsin the dynamic-import path (mm:3211-3273) survived the vocabulary purge — a direct "mechanism in the runtime, vocabulary from the client" violation. Android's runtime is grep-clean of bundler strings. - Rejection detail is debug-only: release builds throw a bare
"Module evaluation promise rejected"(ModuleInternal.mm:1203-1283), violating loud-identical-failures. Android splices path + reason unconditionally. - Boot backstop logs its failures and never evicts (
NativeScript.mm:88-96); its "never settled" fatal string also diverges from the shared contract doc's wording. Android throws both documented fatals and evicts the entry. - Fetch-yield complex (
HttpLoader.mm:897-918,t_bootEvaluationDepth,BootEvalScope):RegisterHttpFetchYieldhas no callers, and the default yield fires after the sync fetch it claims to overlap. Android deleted the whole complex; decide whether the cold-boot placeholder-repaint value is real here or delete likewise. -
importMapobject branch uses the patchable globalJSON.stringify(mm:3441-3448); the C++v8::JSONAPI is tamper-immune (Android switched, with a throwing-toJSONpropagation fix worth mirroring). - The non-pumping TLA wait burns a blind 1ms
usleepper iteration; Android now waits on its event loop's fds (instant wakeup on fetch completions/platform tasks, same 10ms cap). The analogous CFRunLoop-source wait may be worth it. -
import.meta.dirnameis inconsistent with the base-strippedimport.meta.url(Runtime.mm:94-95vs:136-142). - Doc/comment fixes:
getLoadedModuleUrls's URL-like filter and thepreserveQueryFor-before-dev-prefix ordering are implemented identically on both platforms but documented only on Android;primordials.js:5says "second parameter" (it's the fifth); the doc'sinspectquoting example shows single quotes whileinspect.js(byte-identical on both platforms) emitsJSON.stringifydouble quotes. - Port the two pump timer specs from android#1965 (
timer-tlaunderpumpRunLoop: true; the default-options does-not-settle contract spec) — iOS's behavior already matches the agreed contract; the specs just don't exist here.
Shared items tracked elsewhere
The require(esm) facade evaluating under sync-strict (bogus TLA refusal naming ns:require-facade: for createPumpingRequire of a settled TLA module with a default export), the validatePumpingOptions prototype-chain reads, the createRequire file: parser vs node:url disagreement, the JSON-module textual __proto__ wrap, and the per-isolate graph-work counter are shared defects/decisions coordinated from the Android side — see NativeScript/android#2020.
Activity
fix: loader additional hardening (#443) (#444)