[aborovsky]

Problem

On Node.js 26, a finished transfer does not emit end until some other JavaScript callback happens to run, even though libcurl has already received the full response. For a caller that awaits one request at a time this pins every request at libcurl's multi timer period (~1000ms on Linux), while the transfer itself completes in a few milliseconds. Node.js 22/24/25 and macOS were unaffected, which is what made it look intermittent.

Reported in #454.

Root cause

Multi::OnSocket and Multi::OnTimeout enter JS from a libuv callback — that is, from outside any JS context. Multi::CallOnMessageCallback settles the promise that perform() returns from there, and Node.js only performs a microtask checkpoint when a callback scope is closed. Only a HandleScope was being opened, so the continuation that emits end was not scheduled until something else drained the microtask queue — and for a serialized caller, that is the next tick of libcurl's own multi timer.

Before Node.js 26.4.0, an empty callback scope in CheckImmediate ran the microtask queue on every loop iteration, which masked the missing scope. That fast path was removed in nodejs/node#62969 ("src: fast path empty native immediate drain"), surfacing the latent bug.

Multi.addHandle() with Multi.onMessage() is not affected: it invokes the callback directly instead of resolving a promise.

Fix

Open a Napi::CallbackScope in both libuv callbacks (OnSocket / OnTimeout), backed by a single long-lived Napi::AsyncContext per Multi (created in the constructor, reset in Dispose while the environment is still valid). One async context per Multi avoids a napi_async_init on every socket event. Easy::CallSocketEvent, the only other libuv→JS entry point, already goes through MakeCallback with an async context and needs no change.

Test

test/curl/serializedRequests.spec.ts serializes requests against a local server and asserts end fires without waiting for the multi timer. It covers both drain paths:

• the notification-API path (libcurl >= 8.17), and
• the polling fallback, pinned explicitly via new Multi({ shouldUseNotificationsApi: false }).

Both legs fail on the unfixed addon (~1s per request on Node.js 26 / Linux) and pass with the fix.

Notes

This is a port of a fix that has been running in the @brightsec/node-libcurl fork since 5.1.3 (BrightSec#41). Opened as a draft so CI can exercise it across the Node.js 24/25/26 matrix before review.

[JCMais]

hey @aborovsky thanks for the contribution! I am generally aligned with the fix

[JCMais]

@aborovsky I pulled it locally and the fix works, but there is one issue with the async context lifetime that needs to be fixed before we can merge this.

Looks like there is a concrete issue, so thanks for looking into it, on Node.js 26.12.1 (against libcurl 7.81), the repro from #454 never emits end on develop, and emits it in ~4ms with this branch. The new test also fails on develop (~1000ms median) and passes on 24 and 26 with the PR.

The issue tho: Dispose() resets asyncContext, and that can happen while the CallbackScope opened by OnSocket / OnTimeout is still open. That happens if someone calls multi.close() from inside onMessage, which is exactly what we do in examples/04-multi.js. napi_open_callback_scope pushes a pointer to the async context resource into the async resource stack (src/node_api.cc), so after close() anything that reads executionAsyncResource() before the scope closes is reading freed memory. AsyncLocalStorage does that on every getStore() / promise init when using the async_hooks implementation (the default on Node.js 22).

This segfaults every time for me on Node.js 26 (it does not crash on develop):

multi.onMessage((error, handle) => {
  multi.removeHandle(handle)
  handle.close()
  multi.close()
  executionAsyncResource() // segfault
})

On 22 and 24 it does not crash, but it is still a read of freed memory, so pure luck.

The fix is small, keep a reference for the lifetime of the scope, same thing Easy already does with cbOnSocketEventAsyncContext:

-  std::unique_ptr<Napi::AsyncContext> asyncContext;
+  std::shared_ptr<Napi::AsyncContext> asyncContext;

-  if (!obj->asyncContext) return;
+  std::shared_ptr<Napi::AsyncContext> asyncContext = obj->asyncContext;
+  if (!asyncContext) return;

   Napi::Env env = obj->Env();
   Napi::HandleScope handleScope(env);
-  Napi::CallbackScope callbackScope(env, *obj->asyncContext);
+  Napi::CallbackScope callbackScope(env, *asyncContext);

(same thing on OnSocket, and make_unique -> make_shared on the constructor)

And a test for it, this one kills the vitest worker with the current branch and passes with the change above:

test

// examples/04-multi.js closes the Multi from inside onMessage, which runs
// while the callback scope opened by OnSocket / OnTimeout is still open.
// Node.js keeps a pointer into the scope's async context on the async
// resource stack, so that context must outlive the scope even when
// close() resets it. Reading the stack (as AsyncLocalStorage does) used to
// crash the process on Node.js 26.
it('should not crash when the Multi is closed from inside onMessage', async () => {
  const resourceAfterClose = await new Promise<unknown>(
    (resolve, reject) => {
      const multi = new Multi()
      const easy = new Easy()
      withCommonTestOptions(easy)
      easy.setOpt('URL', serverInstance.url)
      easy.setOpt('WRITEFUNCTION', (_buf, size, nmemb) => size * nmemb)

      multi.onMessage((error, handle) => {
        multi.removeHandle(handle)
        handle.close()
        multi.close()

        if (error) {
          reject(error)
          return
        }

        resolve(executionAsyncResource())
      })
      multi.addHandle(easy)
    },
  )

  expect(typeof resourceAfterClose).toBe('object')
})

(needs import { executionAsyncResource } from 'async_hooks' and Easy added to the ../../lib import)

With both changes the full suite passes locally on 24 and 26

Two smaller things:

1. can you add an entry under [Unreleased] -> ### Fixed on the CHANGELOG?
2. this also changes when microtasks / nextTick callbacks queued from inside the data/header/progress callbacks run. Before, they ran after each callback, now they run once when the OnSocket / OnTimeout scope closes. I think that is the correct behavior (it is how Node.js itself does it), but imo worth mentioning in the CHANGELOG entry too.

Can you pull these into the PR?

[aborovsky]

Thanks @JCMais, great catch. I reproduced it locally (macOS arm64, libcurl 8.21) and pushed the fix in 3106150, plus the CHANGELOG entry in 7579a55.

Repro: on Node.js 26.9.0, closing the Multi from inside onMessage (polling path) makes executionAsyncResource() return undefined instead of the resource, and it segfaults every time under Guard Malloc (DYLD_INSERT_LIBRARIES=/usr/lib/libgmalloc.dylib). With std::shared_ptr plus a local copy in OnSocket / OnTimeout, it returns the resource and survives Guard Malloc.

One correction on scope: Node.js 25 is affected too, since &async_context->resource_ is already passed to the callback scope in v25.9.0's node_api.cc. On 22 and 24, though, napi_open_callback_scope passes resource_.Get(isolate), a handle rather than a pointer into the context, so there's no freed read there. That matches what I see: 22 and 24 pass with and without the change, even under Guard Malloc.

Test: I added your test in test/curl/multi.spec.ts, with one change: it uses new Multi({ shouldUseNotificationsApi: false }). With the default Multi on libcurl >= 8.17 (most CI legs), onMessage runs inside libcurl's notify callback, curl_multi_cleanup returns CURLM_RECURSIVE_API_CALL, and assert(code == CURLM_OK) in Dispose() aborts the vitest worker, with or without this PR. That one is already on develop: examples/04-multi.js hits the same assert at the end with libcurl 8.21. Happy to open a separate issue or PR for it if you'd like.

The pinned test fails on the previous commit on 25 and 26 (expected 'undefined' to be 'object') and passes with the fix on 22, 24, 25 and 26.

CHANGELOG: added under [Unreleased] → ### Fixed, including the nextTick / microtask timing note. I confirmed the change: with a header callback that queues both, develop runs header1 tick1 micro1 header2 … and this branch runs header1 header2 … tick1 ….

The full suite passes locally on 24 and 26, as does the #439 stress test. The only local failure is setOpt() > BLOB > should be able to set blob value back to null, which also fails on develop against libcurl 8.21.

CI note: windows-consumer-install / electron-source-install failed on the previous push before compiling any node-libcurl source. MSBuild threw PathTooLongException on node_addon_api_except.vcxproj in the nested consumer path. The job runs on windows-latest, which is now the windows-2025-vs2026 image (MSBuild 18). The workflow last passed on develop on June 4, before GitHub moved that label to VS 2026, so I expect it to fail the same way on this push. Happy to look at it in a separate PR.