Skip to content

Commit 47d8db1

Browse files
committed
fix(linter/plugins): prevent comments being accessed after file is linted (#14727)
Follow on after #14637. I now realize that there was a subtle race condition. We use `astId` counter in the getter for `program.comments`, to check that the getter is not called after that AST is done with. Buffers are reused, so the buffer held in local reference is the same, but the *contents* of the buffer could have changed - it may now contain a *different* AST. Previously, `astId` was incremented in `deserializeProgram`. So if `comments` getter is accessed after another AST is deserialized, it (rightly) throws. But there's a small period of time in between walking the AST on JS side and the *next* call to `deserializeProgram`. During that period, the buffer may be being mutated on Rust side (in another thread), but `astId` hasn't been incremented yet, so the `localAstId === astId` check in `comments` getter will pass, and it won't throw an error as it should. Fix this by incrementing `astId` as soon as traversal of *this* AST ends, instead of before deserializing the *next* AST begins.
1 parent b402024 commit 47d8db1

File tree

4 files changed

+18
-3
lines changed

4 files changed

+18
-3
lines changed

apps/oxlint/src-js/generated/deserialize.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ export function deserializeProgramOnly(buffer, sourceText, sourceByteLen, getLoc
1919
return deserializeWith(buffer, sourceText, sourceByteLen, getLoc, deserializeProgram);
2020
}
2121

22+
export function reset() {
23+
// Increment `astId` counter.
24+
// This prevents `program.comments` being accessed after the AST is done with.
25+
// (see `deserializeProgram`)
26+
astId++;
27+
}
28+
2229
function deserializeWith(buffer, sourceTextInput, sourceByteLenInput, getLocInput, deserialize) {
2330
uint8 = buffer;
2431
uint32 = buffer.uint32;
@@ -42,7 +49,7 @@ function deserializeProgram(pos) {
4249
let refUint32 = uint32,
4350
refUint8 = uint8,
4451
refSourceText = sourceText,
45-
localAstId = ++astId,
52+
localAstId = astId,
4653
end = deserializeU32(pos + 4),
4754
program = parent = {
4855
__proto__: NodeProto,

apps/oxlint/src-js/plugins/source_code.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { DATA_POINTER_POS_32, SOURCE_LEN_OFFSET } from '../generated/constants.j
33
// We use the deserializer which removes `ParenthesizedExpression`s from AST,
44
// and with `range`, `loc`, and `parent` properties on AST nodes, to match ESLint
55
// @ts-expect-error we need to generate `.d.ts` file for this module
6-
import { deserializeProgramOnly } from '../../dist/generated/deserialize.js';
6+
import { deserializeProgramOnly, reset as resetAst } from '../../dist/generated/deserialize.js';
77

88
import visitorKeys from '../generated/keys.js';
99
import {
@@ -78,6 +78,7 @@ export function resetSourceAndAst(): void {
7878
buffer = null;
7979
sourceText = null;
8080
ast = null;
81+
resetAst();
8182
resetLines();
8283
}
8384

crates/oxc_ast/src/serialize/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ impl Program<'_> {
127127
let refUint32 = uint32;
128128
let refUint8 = uint8;
129129
let refSourceText = sourceText;
130-
const localAstId = ++astId;
130+
const localAstId = astId;
131131
/* END_IF */
132132
133133
const start = IS_TS ? 0 : DESER[u32](POS_OFFSET.span.start),

tasks/ast_tools/src/generators/raw_transfer.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,13 @@ fn generate_deserializers(
163163
export function deserializeProgramOnly(buffer, sourceText, sourceByteLen, getLoc) {{
164164
return deserializeWith(buffer, sourceText, sourceByteLen, getLoc, deserializeProgram);
165165
}}
166+
167+
export function reset() {{
168+
// Increment `astId` counter.
169+
// This prevents `program.comments` being accessed after the AST is done with.
170+
// (see `deserializeProgram`)
171+
astId++;
172+
}}
166173
/* END_IF */
167174
168175
function deserializeWith(buffer, sourceTextInput, sourceByteLenInput, getLocInput, deserialize) {{

0 commit comments

Comments
 (0)