diff --git a/lib/runtime-tools/heaptrack-wrapper.ts b/lib/runtime-tools/heaptrack-wrapper.ts index 382cf5187..ac4e8ce34 100644 --- a/lib/runtime-tools/heaptrack-wrapper.ts +++ b/lib/runtime-tools/heaptrack-wrapper.ts @@ -43,7 +43,7 @@ import {PropertyGetter} from '../properties.interfaces.js'; import {BaseRuntimeTool} from './base-runtime-tool.js'; -const O_NONBLOCK = 2048; +const O_NONBLOCK = fsConstants.O_NONBLOCK; export class HeaptrackWrapper extends BaseRuntimeTool { private rawOutput: string; @@ -112,7 +112,7 @@ export class HeaptrackWrapper extends BaseRuntimeTool { return this.execFunc(this.interpreter, [this.rawOutput], execOptions); } - private async finishPipesAndStreams(fd: number, file: WriteStream, socket: net.Socket) { + private async finishPipesAndStreams(fd: number, file: WriteStream, socket: net.Socket): Promise { socket.push(null); await new Promise(resolve => socket.end(() => resolve(true))); @@ -120,6 +120,7 @@ export class HeaptrackWrapper extends BaseRuntimeTool { file.write(Buffer.from([0])); + // Don't manually close fd - the socket owns it and closes it during cleanup if (socket.resetAndDestroy) socket.resetAndDestroy(); socket.unref(); @@ -129,8 +130,6 @@ export class HeaptrackWrapper extends BaseRuntimeTool { resolve(true); }); }); - - return new Promise(resolve => oldfs.close(fd, () => resolve(true))); } private async interpretAndSave(execOptions: ExecutionOptions, result: UnprocessedExecResult) { diff --git a/test/runtime-tools/heaptrack-wrapper-tests.ts b/test/runtime-tools/heaptrack-wrapper-tests.ts new file mode 100644 index 000000000..dce86e3ae --- /dev/null +++ b/test/runtime-tools/heaptrack-wrapper-tests.ts @@ -0,0 +1,82 @@ +// Copyright (c) 2025, Compiler Explorer Authors +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are met: +// +// * Redistributions of source code must retain the above copyright notice, +// this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above copyright +// notice, this list of conditions and the following disclaimer in the +// documentation and/or other materials provided with the distribution. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +// POSSIBILITY OF SUCH DAMAGE. + +import {execSync} from 'node:child_process'; +import * as fs from 'node:fs'; +import * as net from 'node:net'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import {describe, it} from 'vitest'; + +describe('HeaptrackWrapper FD behavior tests', () => { + it('should verify net.Socket properly handles external FDs', async () => { + // This test verifies that net.Socket can handle external FDs and properly goes through its close sequence. + // This is critical for HeaptrackWrapper which creates sockets from pipe FDs. + + if (process.platform === 'win32') { + return; + } + + const tmpDir = os.tmpdir(); + const pipePath = path.join(tmpDir, `test_pipe_${process.pid}_${Date.now()}`); + + try { + // Create a named pipe (exactly like HeaptrackWrapper does) + execSync(`mkfifo "${pipePath}"`); + + // Open the pipe with O_NONBLOCK (like HeaptrackWrapper does) + const O_NONBLOCK = fs.constants.O_NONBLOCK; // Use fs.constants for portability + const O_RDWR = fs.constants.O_RDWR; + const fd = fs.openSync(pipePath, O_RDWR | O_NONBLOCK); + + // Create a net.Socket with the pipe FD (like HeaptrackWrapper) + const socket = new net.Socket({fd: fd, readable: true, writable: true}); + + // Set up close event listener + const closePromise = new Promise(resolve => { + socket.on('close', () => { + // Close event fired - this means the socket went through its complete cleanup sequence + resolve(); + }); + }); + + // Destroy the socket (like HeaptrackWrapper does) + socket.destroy(); + + // Wait for close event to fire + await closePromise; + + // Success! The socket properly handled the external FD and completed its close sequence. We assume + // (based on manual testing) that this includes closing the FD. We NEVER attempt to verify the FD + // state ourselves to avoid FD recycling race conditions. + } finally { + // Clean up the pipe + try { + fs.unlinkSync(pipePath); + } catch { + // Ignore cleanup errors + } + } + }); +});