Skip to content

Commit e2fc6af

Browse files
committed
Fix a use-before-initialize bug in the async compiler
Closes #398
1 parent 4bcd8d5 commit e2fc6af

File tree

2 files changed

+50
-15
lines changed

2 files changed

+50
-15
lines changed

lib/src/compiler/async.ts

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
handleLogEvent,
1717
newCompilePathRequest,
1818
newCompileStringRequest,
19+
promiseWithResolvers,
1920
} from './utils';
2021
import {compilerCommand} from '../compiler-path';
2122
import {activeDeprecationOptions} from '../deprecations';
@@ -128,23 +129,29 @@ export class AsyncCompiler {
128129
);
129130
dispatcher.logEvents$.subscribe(event => handleLogEvent(options, event));
130131

131-
const compilation = new Promise<proto.OutboundMessage_CompileResponse>(
132-
(resolve, reject) =>
133-
dispatcher.sendCompileRequest(request, (err, response) => {
134-
this.compilations.delete(compilation);
135-
// Reset the compilation ID when the compiler goes idle (no active
136-
// compilations) to avoid overflowing it.
137-
// https://github.com/sass/embedded-host-node/pull/261#discussion_r1429266794
138-
if (this.compilations.size === 0) this.compilationId = 1;
139-
if (err) {
140-
reject(err);
141-
} else {
142-
resolve(response!);
143-
}
144-
}),
145-
);
132+
// Avoid `new Promise()` here because `dispatcher.sendCompilerequest` can
133+
// run its callback synchronously, so `compilation` needs to be assigned
134+
// and in `this.compilations` before we call it.
135+
const {
136+
promise: compilation,
137+
resolve,
138+
reject,
139+
} = promiseWithResolvers<proto.OutboundMessage_CompileResponse>();
146140
this.compilations.add(compilation);
147141

142+
dispatcher.sendCompileRequest(request, (err, response) => {
143+
this.compilations.delete(compilation);
144+
// Reset the compilation ID when the compiler goes idle (no active
145+
// compilations) to avoid overflowing it.
146+
// https://github.com/sass/embedded-host-node/pull/261#discussion_r1429266794
147+
if (this.compilations.size === 0) this.compilationId = 1;
148+
if (err) {
149+
reject(err);
150+
} else {
151+
resolve(response!);
152+
}
153+
});
154+
148155
return handleCompileResponse(await compilation);
149156
} finally {
150157
activeDeprecationOptions.delete(optionsKey);

lib/src/compiler/utils.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,3 +228,31 @@ export function handleCompileResponse(
228228
throw utils.compilerError('Compiler sent empty CompileResponse.');
229229
}
230230
}
231+
232+
/**
233+
* A polyfill for the return type of `Promise.withResolvers()` until it's
234+
* universally available in LTS Node.js versions.
235+
*/
236+
interface PromiseWithResolvers<T> {
237+
promise: Promise<T>;
238+
resolve: (value: T | PromiseLike<T>) => void;
239+
// Type definition comes from official types for Node v22.
240+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
241+
reject: (reason?: any) => void;
242+
}
243+
244+
// TODO(nweiz): Replace this with the library function once Node 22 is the
245+
// oldest LTS version (May 2026).
246+
/**
247+
* A polyfill for `Promise.withResolvers()` until it's universally available in
248+
* LTS Node.js versions.
249+
*/
250+
export function promiseWithResolvers<T>(): PromiseWithResolvers<T> {
251+
let resolve: PromiseWithResolvers<T>['resolve'] | undefined;
252+
let reject: PromiseWithResolvers<T>['reject'] | undefined;
253+
const promise = new Promise<T>((resolve_, reject_) => {
254+
resolve = resolve_;
255+
reject = reject_;
256+
});
257+
return {promise, resolve: resolve!, reject: reject!};
258+
}

0 commit comments

Comments
 (0)