Commit 4f2f800
repl: fix FileHandle leak in history initialization
Ensure that the history file handle is closed if initialization fails or
flushing throws an error. This prevents ERR_INVALID_STATE errors where
a FileHandle object is closed during garbage collection.
PR-URL: #61706
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Anna Henningsen <anna@addaleax.net>1 parent 221648a commit 4f2f800
2 files changed
Lines changed: 57 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -327,6 +327,7 @@ class ReplHistory { | |||
| 327 | 327 | ||
| 328 | 328 | await this[kFlushHistory](); | |
| 329 | 329 | } catch (err) { | |
| 330 | + await this[kCloseHandle](); | ||
| 330 | 331 | return this[kHandleHistoryInitError](err, onReadyCallback); | |
| 331 | 332 | } | |
| 332 | 333 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,56 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + // Flags: --expose-internals | ||
| 3 | + | ||
| 4 | + const common = require('../common'); | ||
| 5 | + const fs = require('fs'); | ||
| 6 | + const path = require('path'); | ||
| 7 | + const { ReplHistory } = require('internal/repl/history'); | ||
| 8 | + const assert = require('assert'); | ||
| 9 | + | ||
| 10 | + const tmpdir = require('../common/tmpdir'); | ||
| 11 | + tmpdir.refresh(); | ||
| 12 | + | ||
| 13 | + const historyPath = path.join(tmpdir.path, '.node_repl_history'); | ||
| 14 | + | ||
| 15 | + fs.writeFileSync(historyPath, 'dummy\n'); | ||
| 16 | + | ||
| 17 | + const originalOpen = fs.promises.open; | ||
| 18 | + let closeCalled = false; | ||
| 19 | + | ||
| 20 | + fs.promises.open = async (filepath, flags, mode) => { | ||
| 21 | + const handle = await originalOpen(filepath, flags, mode); | ||
| 22 | + | ||
| 23 | + if (flags === 'r+' && filepath === historyPath) { | ||
| 24 | + handle.truncate = async (len) => { | ||
| 25 | + throw new Error('Mock truncate error'); | ||
| 26 | + }; | ||
| 27 | + | ||
| 28 | + const originalClose = handle.close; | ||
| 29 | + handle.close = async () => { | ||
| 30 | + closeCalled = true; | ||
| 31 | + return originalClose.call(handle); | ||
| 32 | + }; | ||
| 33 | + } | ||
| 34 | + | ||
| 35 | + return handle; | ||
| 36 | + }; | ||
| 37 | + | ||
| 38 | + const context = { | ||
| 39 | + historySize: 30, | ||
| 40 | + on: () => {}, | ||
| 41 | + once: () => {}, | ||
| 42 | + emit: () => {}, | ||
| 43 | + pause: () => {}, | ||
| 44 | + resume: () => {}, | ||
| 45 | + off: () => {}, | ||
| 46 | + line: '', | ||
| 47 | + _historyPrev: () => {}, | ||
| 48 | + _writeToOutput: () => {} | ||
| 49 | + }; | ||
| 50 | + | ||
| 51 | + const history = new ReplHistory(context, { filePath: historyPath }); | ||
| 52 | + | ||
| 53 | + history.initialize(common.mustCall((err) => { | ||
| 54 | + assert.strictEqual(err, null); | ||
| 55 | + assert.strictEqual(closeCalled, true); | ||
| 56 | + })); | ||
0 commit comments