From e2fc6afbf2cab6d4e9c167aedaf036ff2ff12de0 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 19 Nov 2025 13:41:14 -0800 Subject: [PATCH] Fix a use-before-initialize bug in the async compiler Closes #398 --- lib/src/compiler/async.ts | 37 ++++++++++++++++++++++--------------- lib/src/compiler/utils.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 15 deletions(-) diff --git a/lib/src/compiler/async.ts b/lib/src/compiler/async.ts index c41a8015..a746fd79 100644 --- a/lib/src/compiler/async.ts +++ b/lib/src/compiler/async.ts @@ -16,6 +16,7 @@ import { handleLogEvent, newCompilePathRequest, newCompileStringRequest, + promiseWithResolvers, } from './utils'; import {compilerCommand} from '../compiler-path'; import {activeDeprecationOptions} from '../deprecations'; @@ -128,23 +129,29 @@ export class AsyncCompiler { ); dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event)); - const compilation = new Promise( - (resolve, reject) => - dispatcher.sendCompileRequest(request, (err, response) => { - this.compilations.delete(compilation); - // Reset the compilation ID when the compiler goes idle (no active - // compilations) to avoid overflowing it. - // https://github.com/sass/embedded-host-node/pull/261#discussion_r1429266794 - if (this.compilations.size === 0) this.compilationId = 1; - if (err) { - reject(err); - } else { - resolve(response!); - } - }), - ); + // Avoid `new Promise()` here because `dispatcher.sendCompilerequest` can + // run its callback synchronously, so `compilation` needs to be assigned + // and in `this.compilations` before we call it. + const { + promise: compilation, + resolve, + reject, + } = promiseWithResolvers(); this.compilations.add(compilation); + dispatcher.sendCompileRequest(request, (err, response) => { + this.compilations.delete(compilation); + // Reset the compilation ID when the compiler goes idle (no active + // compilations) to avoid overflowing it. + // https://github.com/sass/embedded-host-node/pull/261#discussion_r1429266794 + if (this.compilations.size === 0) this.compilationId = 1; + if (err) { + reject(err); + } else { + resolve(response!); + } + }); + return handleCompileResponse(await compilation); } finally { activeDeprecationOptions.delete(optionsKey); diff --git a/lib/src/compiler/utils.ts b/lib/src/compiler/utils.ts index 7bc0fe94..098753d3 100644 --- a/lib/src/compiler/utils.ts +++ b/lib/src/compiler/utils.ts @@ -228,3 +228,31 @@ export function handleCompileResponse( throw utils.compilerError('Compiler sent empty CompileResponse.'); } } + +/** + * A polyfill for the return type of `Promise.withResolvers()` until it's + * universally available in LTS Node.js versions. + */ +interface PromiseWithResolvers { + promise: Promise; + resolve: (value: T | PromiseLike) => void; + // Type definition comes from official types for Node v22. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + reject: (reason?: any) => void; +} + +// TODO(nweiz): Replace this with the library function once Node 22 is the +// oldest LTS version (May 2026). +/** + * A polyfill for `Promise.withResolvers()` until it's universally available in + * LTS Node.js versions. + */ +export function promiseWithResolvers(): PromiseWithResolvers { + let resolve: PromiseWithResolvers['resolve'] | undefined; + let reject: PromiseWithResolvers['reject'] | undefined; + const promise = new Promise((resolve_, reject_) => { + resolve = resolve_; + reject = reject_; + }); + return {promise, resolve: resolve!, reject: reject!}; +}