Commit 7fb7030
repl: fix flaky test-repl-programmatic-history
The test was flaky because the FileHandle for the history file could
be garbage collected before being explicitly closed, causing
ERR_INVALID_STATE errors when the handle was already closed by the
GC finalizer.
This fix adds an explicit closeHandle() method to ReplHistory and
ensures that REPLServer.close() waits for the history file handle
to be properly closed before completing the close operation.
Refs: nodejs/reliability#1450
PR-URL: #61614
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>1 parent 9409d30 commit 7fb7030
2 files changed
Lines changed: 24 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -58,6 +58,7 @@ const kResolveHistoryPath = Symbol('_kResolveHistoryPath'); | |||
| 58 | 58 | const kReplHistoryMessage = Symbol('_kReplHistoryMessage'); | |
| 59 | 59 | const kFlushHistory = Symbol('_kFlushHistory'); | |
| 60 | 60 | const kGetHistoryPath = Symbol('_kGetHistoryPath'); | |
| 61 | + const kCloseHandle = Symbol('_kCloseHandle'); | ||
| 61 | 62 | ||
| 62 | 63 | class ReplHistory { | |
| 63 | 64 | constructor(context, options) { | |
@@ -393,15 +394,29 @@ class ReplHistory { | |||
| 393 | 394 | } | |
| 394 | 395 | this[kContext].off('line', this[kOnLine].bind(this)); | |
| 395 | 396 | ||
| 397 | + await this[kCloseHandle](); | ||
| 398 | + } | ||
| 399 | + | ||
| 400 | + async [kCloseHandle]() { | ||
| 396 | 401 | if (this[kHistoryHandle] !== null) { | |
| 402 | + const handle = this[kHistoryHandle]; | ||
| 403 | + this[kHistoryHandle] = null; | ||
| 397 | 404 | try { | |
| 398 | - await this[kHistoryHandle].close(); | ||
| 405 | + await handle.close(); | ||
| 399 | 406 | } catch (err) { | |
| 400 | 407 | debug('Error closing history file:', err); | |
| 401 | 408 | } | |
| 402 | 409 | } | |
| 403 | 410 | } | |
| 404 | 411 | ||
| 412 | + /** | ||
| 413 | + * Closes the history file handle. | ||
| 414 | + * @returns {Promise<void>} | ||
| 415 | + */ | ||
| 416 | + closeHandle() { | ||
| 417 | + return this[kCloseHandle](); | ||
| 418 | + } | ||
| 419 | + | ||
| 405 | 420 | [kReplHistoryMessage]() { | |
| 406 | 421 | if (this[kHistory].length === 0) { | |
| 407 | 422 | ReplHistory[kWriteToOutput]( | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1039,12 +1039,18 @@ class REPLServer extends Interface { | |||
| 1039 | 1039 | this[kBufferedCommandSymbol] = ''; | |
| 1040 | 1040 | } | |
| 1041 | 1041 | close() { | |
| 1042 | - if (this.terminal && this.historyManager.isFlushing && !this._closingOnFlush) { | ||
| 1042 | + if (this.terminal && this.historyManager?.isFlushing && !this._closingOnFlush) { | ||
| 1043 | 1043 | this._closingOnFlush = true; | |
| 1044 | - this.once('flushHistory', () => super.close()); | ||
| 1044 | + this.once('flushHistory', () => this.close()); | ||
| 1045 | 1045 | ||
| 1046 | 1046 | return; | |
| 1047 | 1047 | } | |
| 1048 | + // Ensure the history file handle is closed before completing | ||
| 1049 | + if (this.terminal && this.historyManager?.closeHandle && !this._historyHandleClosed) { | ||
| 1050 | + this._historyHandleClosed = true; | ||
| 1051 | + this.historyManager.closeHandle().then(() => super.close()); | ||
| 1052 | + return; | ||
| 1053 | + } | ||
| 1048 | 1054 | process.nextTick(() => super.close()); | |
| 1049 | 1055 | } | |
| 1050 | 1056 | createContext() { | |
0 commit comments