Skip to content

Commit 084f93f

Browse files
glasstigerclaude
andcommitted
Null loaded dict entries after a failed truncate
PersistedSymbolDict.openExisting frees the loaded-entries buffer in the truncate-failure branch, then calls ff.close(fd). If that close threw, the enclosing catch re-freed the same buffer -- a double-free -- because, unlike the bad-magic branch, this branch did not null the pointer first. Null entriesAddr/entriesLen right after freeing them, matching the buf discipline above, so the catch's guard skips the second free. Correct the catch comment, which wrongly claimed nothing between the malloc and the return could throw -- the truncate branch's free-then-close is exactly such a point. This is defensive hardening: FilesFacade.close returns a status code and never throws (the native close never raises), so the path is unreachable today, but the nulling keeps it safe against a future facade or edit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 76fe4f1 commit 084f93f

1 file changed

Lines changed: 9 additions & 4 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/PersistedSymbolDict.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,10 @@ private static PersistedSymbolDict openExisting(FilesFacade ff, String filePath,
608608
LOG.warn("symbol dict {} could not drop its torn/stale tail (truncate failed); recreating", filePath);
609609
if (entriesAddr != 0L) {
610610
Unsafe.free(entriesAddr, entriesLen, MemoryTag.NATIVE_DEFAULT);
611+
// Null after freeing (like buf above) so the catch below cannot
612+
// double-free entriesAddr if the following ff.close throws.
613+
entriesAddr = 0L;
614+
entriesLen = 0;
611615
}
612616
ff.close(fd);
613617
return null;
@@ -617,10 +621,11 @@ private static PersistedSymbolDict openExisting(FilesFacade ff, String filePath,
617621
if (buf != 0L) {
618622
Unsafe.free(buf, (int) fileLen, MemoryTag.NATIVE_DEFAULT);
619623
}
620-
// entriesAddr is transferred to the returned dict on the success path,
621-
// and the catch only runs before that return, so freeing it here cannot
622-
// double-free. Unreachable today (nothing between its malloc and the
623-
// return throws), but keeps the error path leak-free against a future edit.
624+
// Free entriesAddr if it was allocated and not yet handed off. The success
625+
// path transfers it to the returned dict, and every path that frees it
626+
// earlier (the truncate-failure branch above) also nulls it, so this cannot
627+
// double-free. Keeps the error path leak-free on any throw between its
628+
// malloc and the return.
624629
if (entriesAddr != 0L) {
625630
Unsafe.free(entriesAddr, entriesLen, MemoryTag.NATIVE_DEFAULT);
626631
}

0 commit comments

Comments
 (0)