Fix use-after-free in XMLHttpRequest::Send continuation

[bkaradzic-microsoft]

Problem

XMLHttpRequest::Send schedules its completion continuation capturing this raw, with arcana::cancellation::none() and no lifetime guarantee:

m_request.SendAsync()
    .then(arcana::inline_scheduler, ..., [sendRegion]{ ... })
    .then(m_runtimeScheduler, arcana::cancellation::none(), [this](const arcana::expected<void, std::exception_ptr>& result) {
        const auto statusCode = arcana::underlying_cast(m_request.StatusCode());  // <-- AV on freed `this`
        SetReadyState(ReadyState::Done);
        ...
        m_eventHandlerRefs.clear();
    });

Nothing keeps the JS XMLHttpRequest wrapper alive across the async window. If the wrapper is garbage-collected while the request is in flight (the requesting script dropped its reference, the scene/loader finished), the wrapper and the C++ object behind it are destroyed before the continuation runs on the runtime scheduler. Dereferencing m_request / m_eventHandlerRefs / m_readyState then faults.

Observed as a flaky access violation in CI (BabylonNative), stack top:

0: arcana::internal::output_wrapper<void,std::exception_ptr>::invoke<`Babylon::Polyfills::Internal::XMLHttpRequest::Send'::`2'::<lambda_2> &, ...>
1: ...task<void,std::exception_ptr>::then<JsRuntimeScheduler, ...XMLHttpRequest::Send'::`2'::<lambda_2> >...

i.e. the [this] completion lambda running on a dead this.

Fix

Anchor the wrapper for the duration of the request with a strong Napi::ObjectReference held in a shared_ptr and captured by the continuation. This is the same idiom already used in FileReader (see the anchor in Polyfills/File/Source/FileReader.cpp). The anchor is released automatically when the request settles and the lambda is destroyed — no member self-reference, no extra teardown path, and the continuation capture stays small (arcana sizes the task payload to the callable).

Validation

• XMLHttpRequest polyfill compiles clean against napi + arcana.
• Built BabylonNative Playground against the patched polyfill and ran the XHR-heavy asset-loading tests ("Roundtrip babylon file …") headless: ran=2 passed=2 failed=0, exit 0 — no regression.

The original crash is a GC/teardown-timing race, so it reproduces only intermittently; the fix removes the race by construction (the wrapper cannot be collected before the continuation runs).

[Copilot]

Pull request overview

This PR addresses a use-after-free in the XMLHttpRequest::Send async completion path by ensuring the JS wrapper (and thus the underlying Napi::ObjectWrap) remains alive until the request settles.

Changes:

• Add a strong Napi::ObjectReference “anchor” stored in a std::shared_ptr and captured by the async completion continuation to prevent GC from collecting the wrapper mid-request.
• Document the rationale inline and align the approach with the existing FileReader anchoring idiom.

Comments suppressed due to low confidence (1)

Polyfills/XMLHttpRequest/Source/XMLHttpRequest.cpp:282

• The completion continuation captures anchor only to extend the wrapper lifetime, but never uses it. With warnings_as_errors(XMLHttpRequest) enabled, many compilers warn on unused lambda captures (e.g. -Wunused-lambda-capture under -Wall), which can break the build. Make the intent explicit by marking the capture as used inside the lambda body.

            .then(m_runtimeScheduler, arcana::cancellation::none(), [this, anchor{std::move(anchor)}](const arcana::expected<void, std::exception_ptr>& result) {
                // Run on every outcome -- transport exception OR underlying request succeeded but ended in a non-2xx
                // status (e.g. a missing local file on UWP, where UrlLib silently retains status 0). The previous
                // success-only continuation here skipped readyState=Done / loadend / error and let the JS observer
                // hang.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.