mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-11 03:38:07 +00:00
fix(lbug): skip native close on CLI exit to dodge LadybugDB destructor double-free (#2264)
THE actual fix for the `analyze --pdg` crash. gdb shows the abort is a double-free
inside LadybugDB's own destructor during conn.close():
"double free or corruption (out)" -> abort
lbug::main::ClientContext::~ClientContext()
lbug::main::Connection::~Connection()
NodeConnection::Close(...) <- conn.close() from safeClose
It reproduces with the WAL driver OFF and with serial load, so it is NOT the
checkpoint/COPY concurrency the rest of this branch serialized — it's a native
LadybugDB engine bug (@ladybugdb/core 0.17.1, latest stable) triggered by the
larger --pdg write set, firing during teardown AFTER a fully-written, checkpointed
index.
Fix: closeLbug({ skipNativeClose }) CHECKPOINTs for durability (flushWAL) then
skips conn.close()/db.close(), leaving the handles referenced so no GC finalizer
re-runs the destructor. The CLI analyze command (success, error, and SIGINT paths
all process.exit) opts in via skipNativeCloseOnExit; long-lived callers (MCP
server, tests) keep the real close. Mirrors the pool adapter's fire-and-forget
native close and the ONNX native-cleanup philosophy.
Validated end-to-end: `analyze --pdg --force` now exits 0 with a 193,876-node
index; re-opening it (no --force) reads clean and reports up-to-date, proving the
CHECKPOINT-only persistence is durable without db.close().
Workaround for an upstream LadybugDB bug (ClientContext destructor double-free).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JBJomjoTdBV2eveDVq4JMm
This commit is contained in:
parent
a9cde029d9
commit
a84645a4b1
4 changed files with 55 additions and 4 deletions
|
|
@ -1168,7 +1168,10 @@ const analyzeCommandImpl = async (
|
|||
aborted = true;
|
||||
bar.stop();
|
||||
console.log('\n Interrupted — cleaning up...');
|
||||
closeLbug()
|
||||
// process.exit(130) follows, so skip the native close (LadybugDB destructor
|
||||
// can double-free after --pdg writes, #2264); the CHECKPOINT inside closeLbug
|
||||
// still flushes the WAL.
|
||||
closeLbug({ skipNativeClose: true })
|
||||
.catch(() => {})
|
||||
.finally(async () => {
|
||||
const { flushLoggerSync } = await import('../core/logger.js');
|
||||
|
|
@ -1273,6 +1276,12 @@ const analyzeCommandImpl = async (
|
|||
// Extra fetch-wrapper names from `.gitnexusrc` (#1589/#1852 residual);
|
||||
// forwarded to the routes phase consumer scan.
|
||||
fetchWrappers: options.fetchWrappers,
|
||||
// The CLI always process.exit()s after this returns (success path at the
|
||||
// end of analyzeCommandImpl, error/interrupt paths via process.exit too),
|
||||
// so the finalize close skips the native conn/db close — it can double-free
|
||||
// in LadybugDB's ClientContext destructor after --pdg writes (#2264). The
|
||||
// CHECKPOINT keeps the index durable; process exit reclaims the handles.
|
||||
skipNativeCloseOnExit: true,
|
||||
},
|
||||
{
|
||||
onProgress: (_phase, percent, message) => {
|
||||
|
|
|
|||
|
|
@ -1900,7 +1900,23 @@ export const safeClose = async (): Promise<void> => {
|
|||
}
|
||||
};
|
||||
|
||||
export const closeLbug = async (): Promise<void> => {
|
||||
export const closeLbug = async (options: { skipNativeClose?: boolean } = {}): Promise<void> => {
|
||||
if (options.skipNativeClose) {
|
||||
// The caller is about to exit the process (CLI analyze success/error/SIGINT
|
||||
// all end in process.exit). CHECKPOINT for durability, then DELIBERATELY skip
|
||||
// conn.close()/db.close(): LadybugDB's ClientContext/Connection destructor can
|
||||
// double-free after large --pdg writes (gdb: `double free or corruption` in
|
||||
// ClientContext::~ClientContext via NodeConnection::Close), aborting the
|
||||
// process AFTER a fully-written, checkpointed index. flushWAL above already
|
||||
// persisted the data; process exit reclaims the native handles. We leave
|
||||
// conn/db referenced and module state intact so a GC finalizer cannot run the
|
||||
// same destructor before exit, and any post-analyze read reuses the live
|
||||
// connection. Mirrors the pool adapter's fire-and-forget native close
|
||||
// (pool-adapter.ts) and the ONNX native-cleanup philosophy. This is a
|
||||
// workaround for a LadybugDB engine bug — see the upstream report.
|
||||
await flushWAL();
|
||||
return;
|
||||
}
|
||||
await safeClose();
|
||||
currentDbPath = null;
|
||||
ftsLoaded = false;
|
||||
|
|
|
|||
|
|
@ -232,6 +232,15 @@ export interface AnalyzeOptions {
|
|||
* consumer scan unchanged.
|
||||
*/
|
||||
fetchWrappers?: string[];
|
||||
/**
|
||||
* The caller will `process.exit()` immediately after this analyze returns (the
|
||||
* CLI `analyze` command). When set, the finalize/error close CHECKPOINTs for
|
||||
* durability but skips the native `conn.close()`/`db.close()`, which can
|
||||
* double-free in LadybugDB's `ClientContext` destructor after large `--pdg`
|
||||
* writes (gdb-confirmed) — aborting the process AFTER a fully-written index.
|
||||
* Process exit reclaims the handles. Long-lived callers (MCP server, tests)
|
||||
* leave this unset so they get a real close. See `closeLbug`. */
|
||||
skipNativeCloseOnExit?: boolean;
|
||||
}
|
||||
|
||||
export interface AnalyzeResult {
|
||||
|
|
@ -1549,7 +1558,10 @@ export async function runFullAnalysis(
|
|||
// Stop the manual checkpoint driver before closeLbug so its
|
||||
// in-flight CHECKPOINT cannot race the `safeClose` CHECKPOINT.
|
||||
await walCheckpointDriver.stop();
|
||||
await closeLbug();
|
||||
// CLI callers (about to process.exit) skip the native close to dodge a
|
||||
// LadybugDB destructor double-free after --pdg writes — the CHECKPOINT inside
|
||||
// closeLbug keeps the index durable (#2264). Long-lived callers close for real.
|
||||
await closeLbug({ skipNativeClose: options.skipNativeCloseOnExit });
|
||||
|
||||
progress('done', 100, 'Done');
|
||||
|
||||
|
|
@ -1570,7 +1582,9 @@ export async function runFullAnalysis(
|
|||
/* swallow — surface path is the rethrow below */
|
||||
}
|
||||
try {
|
||||
await closeLbug();
|
||||
// Same native-close skip on the error path for CLI callers — they
|
||||
// process.exit too, and the native close can double-free (#2264).
|
||||
await closeLbug({ skipNativeClose: options.skipNativeCloseOnExit });
|
||||
} catch {
|
||||
/* swallow */
|
||||
}
|
||||
|
|
|
|||
|
|
@ -72,6 +72,18 @@ withTestLbugDB('conn-serialization', () => {
|
|||
expect(lockSpy.mock.calls.length).toBeGreaterThanOrEqual(filePathTables.length);
|
||||
expect(result).toMatchObject({ deletedNodes: 0 });
|
||||
});
|
||||
|
||||
it('closeLbug({ skipNativeClose }) checkpoints but leaves the connection open (#2264 close-crash)', async () => {
|
||||
const adapter = await import('../../src/core/lbug/lbug-adapter.js');
|
||||
await adapter.closeLbug({ skipNativeClose: true });
|
||||
// The native conn/db are deliberately NOT torn down — that avoids LadybugDB's
|
||||
// ClientContext destructor double-free after --pdg writes. The connection
|
||||
// stays ready and queryable (the CHECKPOINT made the index durable; process
|
||||
// exit reclaims the handles on the CLI path).
|
||||
expect(adapter.isLbugReady()).toBe(true);
|
||||
const rows = await adapter.executeQuery('RETURN 1 AS one');
|
||||
expect(rows).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue