diff --git a/src/domain/javascript/javascriptBuiltinResourceMethods.ts b/src/domain/javascript/javascriptBuiltinResourceMethods.ts index 8034b902fe..95f66f250b 100644 --- a/src/domain/javascript/javascriptBuiltinResourceMethods.ts +++ b/src/domain/javascript/javascriptBuiltinResourceMethods.ts @@ -3,9 +3,32 @@ export const BUILTIN_RESOURCE_METHODS = { fs: ["open", "openSync", "createReadStream", "createWriteStream"], "fs/promises": ["open"], net: ["connect", "createConnection"], - tls: ["connect", "createConnection"], + tls: ["connect"], } as const; +/** + * Release methods the value Node returns from each acquisition actually has. + * + * `fs.openSync` returns a numeric descriptor and `fs.open` in its callback form + * returns `undefined`, so neither has a release method at all; the descriptor is + * released through `fs.closeSync`/`fs.close`, which is not a member call. A + * `fs.ReadStream` exposes `end` as its byte-offset field, not a method. A + * `FileHandle` and a `net.Socket`/`TLSSocket` each expose only their own subset, + * so a family-blind release list claims a teardown that throws instead. + */ +export const BUILTIN_RESOURCE_RELEASE_METHODS = { + "fs:open": [], + "fs:openSync": [], + "fs:createReadStream": ["close", "destroy"], + "fs:createWriteStream": ["close", "destroy", "end"], + "fs/promises:open": ["close"], + "net:connect": ["destroy", "end"], + "net:createConnection": ["destroy", "end"], + "tls:connect": ["destroy", "end"], +} as const satisfies Readonly< + Record +>; + export const BUILTIN_RESOURCE_SPECIFIERS = Object.keys( BUILTIN_RESOURCE_METHODS, ); diff --git a/src/domain/javascript/javascriptSemanticAnalysis.release_families.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.release_families.test.ts new file mode 100644 index 0000000000..e7b7aae52f --- /dev/null +++ b/src/domain/javascript/javascriptSemanticAnalysis.release_families.test.ts @@ -0,0 +1,134 @@ +import { expect, it } from "vitest"; + +import { analyzeJavaScriptSemantics } from "./javascriptSemanticAnalysis.js"; + +/** + * Node's own objects expose different teardown members per acquisition, so a + * member call that Node rejects must not be reported as a release. Every row is + * a pair that throws at the call site on Node 24.22.1. + */ +const rejected = [ + [ + "a numeric descriptor from fs.openSync", + `import { openSync } from "node:fs"; + const fd = openSync("input.txt", "r"); + fd.close();`, + ], + [ + "a numeric descriptor released with destroy", + `import { openSync } from "node:fs"; + const fd = openSync("input.txt", "r"); + fd.destroy();`, + ], + [ + "the callback form of fs.open", + `import { open } from "node:fs"; + const pending = open("input.txt", "r", () => {}); + pending.close();`, + ], + [ + "a read stream's byte-offset field", + `import { createReadStream } from "node:fs"; + const stream = createReadStream("input.txt"); + stream.end();`, + ], + [ + "a FileHandle without a release member", + `import { open } from "node:fs/promises"; + const handle = await open("input.txt"); + handle.destroy();`, + ], + [ + "a socket without close", + `import { connect } from "node:net"; + const socket = connect(9); + socket.close();`, + ], + [ + "a TLS socket without close", + `import { connect } from "node:tls"; + const socket = connect(9); + socket.close();`, + ], +]; + +it.each(rejected)("emits no release for %s", (_name, source) => { + expect( + analyzeJavaScriptSemantics(source) + .resourceOperations.filter(({ kind }) => kind === "release") + .map(({ method }) => method), + ).toEqual([]); +}); + +/** The pairs Node does support stay complete, so the filter is pure narrowing. */ +const accepted = [ + [ + "a FileHandle closed", + `import { open } from "node:fs/promises"; + const handle = await open("input.txt"); + handle.close();`, + "close", + ], + [ + "a read stream destroyed", + `import { createReadStream } from "node:fs"; + const stream = createReadStream("input.txt"); + stream.destroy();`, + "destroy", + ], + [ + "a socket ended", + `import { connect } from "node:net"; + const socket = connect(9); + socket.end();`, + "end", + ], +]; + +it.each(accepted)("keeps %s complete", (_name, source, method) => { + expect( + analyzeJavaScriptSemantics(source) + .resourceOperations.filter(({ kind }) => kind === "release") + .map(({ method: released, resolution }) => ({ + method: released, + resolution, + })), + ).toEqual([{ method, resolution: "complete" }]); +}); + +/** + * A binding that can hold more than one acquisition stays partial even when a + * method is admissible for one of them: the release is only certain if the + * binding held exactly one acquisition before the filter ran. + */ +it.each([ + [ + "a FileHandle or a socket, released with close", + `import { open } from "node:fs/promises"; + import { connect } from "node:net"; + let handle; + if (flag) { handle = await open("input.txt"); } + else { handle = connect(9); } + handle.close();`, + [{ method: "close", linked: 1, resolution: "partial" }], + ], + [ + "a read stream or a write stream, released with end", + `import { createReadStream, createWriteStream } from "node:fs"; + let stream; + if (flag) { stream = createReadStream("input.txt"); } + else { stream = createWriteStream("output.txt"); } + stream.end();`, + [{ method: "end", linked: 1, resolution: "partial" }], + ], +])("keeps %s partial", (_name, source, expected) => { + expect( + analyzeJavaScriptSemantics(source) + .resourceOperations.filter(({ kind }) => kind === "release") + .map(({ method, linkedResourceIds, resolution }) => ({ + method, + linked: linkedResourceIds.length, + resolution, + })), + ).toEqual(expected); +}); diff --git a/src/domain/javascript/javascriptSemanticResources.ts b/src/domain/javascript/javascriptSemanticResources.ts index 7ca9420076..ec5e5252f4 100644 --- a/src/domain/javascript/javascriptSemanticResources.ts +++ b/src/domain/javascript/javascriptSemanticResources.ts @@ -3,6 +3,7 @@ import * as t from "@babel/types"; import { BUILTIN_RESOURCE_SPECIFIERS, BUILTIN_RESOURCE_METHODS, + BUILTIN_RESOURCE_RELEASE_METHODS, } from "./javascriptBuiltinResourceMethods.js"; import type { JavaScriptSemanticCallable, @@ -29,8 +30,23 @@ import { range } from "./javascriptStaticAnalysisHelpers.js"; const RELEASE_METHODS = ["close", "destroy", "end"] as const; +/** + * Release methods the value from this acquisition actually has. An unlisted + * specifier/method pair admits none, so a call to it emits no release fact. + */ +const releaseMethodsFor = ( + acquisition: ResourceCandidate, +): readonly ("close" | "destroy" | "end")[] => { + const key = `${acquisition.specifier}:${acquisition.method}`; + const table: Readonly< + Record + > = BUILTIN_RESOURCE_RELEASE_METHODS; + return table[key] ?? []; +}; + interface ResourceCandidate { readonly node: t.CallExpression | t.OptionalCallExpression; + readonly specifier: string; readonly method: JavaScriptSemanticResourceOperation["method"]; readonly ownerCallableId: string | null; readonly resultBindingId: string | null; @@ -62,11 +78,12 @@ const collectAcquisitions = ( const output: ResourceCandidate[] = []; traverseDataEffects(program, context, (node) => { if (!t.isCallExpression(node) && !t.isOptionalCallExpression(node)) return; - const method = acquisitionMethod(node.callee, context.state); - if (method === null) return; + const reference = acquisitionMethod(node.callee, context.state); + if (reference === null) return; output.push({ node, - method, + specifier: reference.specifier, + method: reference.method, ownerCallableId: context.callableStack.at(-1) ?? null, resultBindingId: outerDataEffectBinding(node, context), }); @@ -102,9 +119,15 @@ const collectReleases = ( member.object, member.object.name, ); - const linked = + const candidates = binding === undefined ? [] : (byBinding.get(binding.bindingId) ?? []); - if (binding === undefined || linked.length === 0) return; + if (binding === undefined || candidates.length === 0) return; + const linked = candidates.filter((acquisition) => + releaseMethodsFor(acquisition).includes(method), + ); + // No linked acquisition exposes this release method, so the call is not a + // release of it at all: Node throws at the call site instead. + if (linked.length === 0) return; output.push({ resourceId: `resource:release:${String(node.start ?? -1)}:${String(node.end ?? -1)}`, kind: "release", @@ -169,10 +192,15 @@ const immutableAcquisition = ( resolution: "complete", }); +interface AcquisitionReference { + readonly specifier: string; + readonly method: JavaScriptSemanticResourceOperation["method"]; +} + const acquisitionMethod = ( callee: t.Node, state: JavaScriptSemanticAnalysisState, -): JavaScriptSemanticResourceOperation["method"] | null => { +): AcquisitionReference | null => { for (const specifier of BUILTIN_RESOURCE_SPECIFIERS) { const method = builtinDataEffectMethod(callee, state, [specifier]); if ( @@ -183,7 +211,10 @@ const acquisitionMethod = ( ] as readonly string[] ).includes(method) ) - return method as JavaScriptSemanticResourceOperation["method"]; + return { + specifier, + method: method as JavaScriptSemanticResourceOperation["method"], + }; } return null; };