core/src/main/java/io/questdb/std/MmapCache.java (1)

287-296: Asserting munmap failures is good; add a release-path signal and a len>0 assert.

The early return on address==0 and conditional accounting are solid. With assertions disabled in production, a failed munmap silently does nothing and leaks accounting. Consider emitting a log/warn (or metrics counter) on failure, and assert len>0 to catch misuse earlier.

Example minimal tweak:

 private static void unmap0(long address, long len, int memoryTag) {
     if (address == 0) {
         return;
     }
+    assert len > 0 : "munmap length must be > 0 [len=" + len + ']';
     int result = Files.munmap0(address, len);
     if (result != -1) {
         Unsafe.recordMemAlloc(-len, memoryTag);
     }
     assert result != -1 :
             "failed to munmap non-zero address [address=" + address + ", len=" + len + ", memoryTag=" + memoryTag + ']';
+    // Optional: emit a release-path signal when assertions are off.
+    // e.g., LOG.warn().$("munmap failed [address=").$(address).$(", len=").$(len).$(", tag=").$(memoryTag).I$();
 }

core/src/main/java/io/questdb/griffin/engine/table/parquet/MappedMemoryPartitionDescriptor.java (2)

55-59: Use overflow-safe sizing and add a sanity check before subtracting HEADER_SIZE.

Casting symbolOffsetsSize to int can overflow silently. Also, subtracting HEADER_SIZE assumes symbolOffsetsAddr ≥ HEADER_SIZE.

-            if (symbolOffsetsAddr != 0) {
-                final long symbolOffsetsSize = columnData.get(rawIndex + SYMBOL_OFFSET_SIZE_OFFSET);
-                final long offsetsMemSize = SymbolMapWriter.keyToOffset((int) symbolOffsetsSize + 1);
-                ff.munmap(symbolOffsetsAddr - SymbolMapWriter.HEADER_SIZE, offsetsMemSize, MemoryTag.MMAP_PARQUET_PARTITION_CONVERTER);
-            }
+            if (symbolOffsetsAddr != 0) {
+                final long symbolOffsetsSize = columnData.get(rawIndex + SYMBOL_OFFSET_SIZE_OFFSET);
+                // Ensure address math is valid and sizing is safe.
+                assert symbolOffsetsAddr >= SymbolMapWriter.HEADER_SIZE;
+                final int keyCapacity = Math.toIntExact(symbolOffsetsSize + 1);
+                final long offsetsMemSize = SymbolMapWriter.keyToOffset(keyCapacity);
+                ff.munmap(symbolOffsetsAddr - SymbolMapWriter.HEADER_SIZE, offsetsMemSize, MemoryTag.MMAP_PARQUET_PARTITION_CONVERTER);
+            }

---

55-59: Optional: reduce repetition of MemoryTag.

Capture MemoryTag.MMAP_PARQUET_PARTITION_CONVERTER in a local final int tag to make the loop a touch cleaner.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

core/src/main/c/share/dedup.cpp (1)

635-643: Optional: express the null-scan with std::any_of for clarity.

Same behavior, slightly tighter and idiomatic.

-                [&](int data_index, int64_t lo, int64_t hi) -> bool {
-                    auto data = data_ptrs[data_index];
-                    for (int64_t i = lo; i < hi; i++) {
-                        if (data[i] != null_value) {
-                            return true;
-                        }
-                    }
-                    return false;
-                }
+                [&](int data_index, int64_t lo, int64_t hi) -> bool {
+                    const T* data = data_ptrs[data_index];
+                    return std::any_of(data + lo, data + hi,
+                                       [&](const T &v){ return v != null_value; });
+                }

core/src/main/java/io/questdb/cutlass/text/CopyTask.java (1)

1342-1344: Defensive reset after unmap

Set the local to 0 after unmapping to prevent accidental reuse.

-                if (mergeIndexAddr != 0) {
-                    ff.munmap(mergeIndexAddr, mergedIndexSize, MemoryTag.MMAP_IMPORT);
-                }
+                if (mergeIndexAddr != 0) {
+                    ff.munmap(mergeIndexAddr, mergedIndexSize, MemoryTag.MMAP_IMPORT);
+                    mergeIndexAddr = 0; // defensive
+                }

core/src/main/java/io/questdb/griffin/engine/functions/table/ReadParquetFunctionFactory.java (1)

100-102: Minor consistency/defense

Reset addr after unmap.

-                if (addr != 0) {
-                    ff.munmap(addr, fileSize, MemoryTag.MMAP_PARQUET_PARTITION_DECODER);
-                }
+                if (addr != 0) {
+                    ff.munmap(addr, fileSize, MemoryTag.MMAP_PARQUET_PARTITION_DECODER);
+                    addr = 0;
+                }

.github/workflows/rebuild_native_libs.yml (1)

33-35: Make cmake uninstall resilient

Avoid failures if cmake is absent or has dependencies.

-          # https://github.com/actions/runner-images/issues/12912
-          # Temporary fix (mtopolnik 2025-09-02): GH Action pre-installs a custom cmake version, install then fails
-          brew uninstall cmake
+          # https://github.com/actions/runner-images/issues/12912
+          # Temporary fix (mtopolnik 2025-09-02): GH Action pre-installs a custom cmake version, install then fails
+          if brew list --versions cmake >/dev/null 2>&1; then
+            brew uninstall --ignore-dependencies cmake || true
+          fi

core/src/main/c/windows/files.c (1)

703-711: Also guard negative len/offset (paranoia, matches Java checks)

Native layer can mirror Java validations for robustness.

     if (len == 0) {
         // With len == 0, Windows will mmap the whole file. To be POSIX-compatible, return MAP_FAILED instead.
         // docs: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-mapviewoffileex
         // dwNumberOfBytesToMap: "If this parameter is 0 (zero), the mapping extends from the
         // specified offset to the end of the file mapping."
         SetLastError(ERROR_INVALID_PARAMETER);
         SaveLastError();
         return -1;
     }
+    if (len < 0 || offset < 0) {
+        SetLastError(ERROR_INVALID_PARAMETER);
+        SaveLastError();
+        return -1;
+    }

core/src/main/java/io/questdb/cairo/O3PartitionJob.java (2)

267-269: Reset address after unmap

Small defensive cleanup.

-                if (parquetAddr != 0) {
-                    ff.munmap(parquetAddr, parquetSize, MemoryTag.MMAP_PARQUET_PARTITION_DECODER);
-                }
+                if (parquetAddr != 0) {
+                    ff.munmap(parquetAddr, parquetSize, MemoryTag.MMAP_PARQUET_PARTITION_DECODER);
+                    parquetAddr = 0;
+                }

---

2570-2572: Nit: reset address; plus consider zero-size fast-path

• Reset local after unmap.
• Optionally skip mapping when newParquetSize == 0 to avoid needless exceptions.

-            if (parquetAddr != 0) {
-                ff.munmap(parquetAddr, newParquetSize, MemoryTag.MMAP_PARQUET_PARTITION_DECODER);
-            }
+            if (parquetAddr != 0) {
+                ff.munmap(parquetAddr, newParquetSize, MemoryTag.MMAP_PARQUET_PARTITION_DECODER);
+                parquetAddr = 0;
+            }

Additional (outside this hunk) suggestion:

// early in updateParquetIndexes, before mapping
if (newParquetSize == 0) {
    return;
}

core/src/main/java/io/questdb/std/MmapCache.java (8)

51-58: Prefer errnoInvalidParameter over errno=0 for invalid len.

Use the dedicated errno to improve diagnostics and align with cross-platform consistency goals.

-        if (len < 1) {
-            throw CairoException.critical(0)
+        if (len < 1) {
+            throw CairoException.critical(Files.errnoInvalidParameter())
                     .put("could not mmap file, invalid len [len=").put(len)
                     .put(", offset=").put(offset)
                     .put(", fd=").put(fd)
                     .put(", memoryTag=").put(memoryTag)
                     .put(']');
         }

---

112-121: Use errnoInvalidParameter for invalid newSize.

Same rationale as for len above.

-        if (newSize < 1) {
-            throw CairoException.critical(0)
+        if (newSize < 1) {
+            throw CairoException.critical(Files.errnoInvalidParameter())
                     .put("could not remap file, invalid newSize [previousSize=").put(previousSize)
                     .put(", newSize=").put(newSize)
                     .put(", offset=").put(offset)
                     .put(", fd=").put(fd)
                     .put(", memoryTag=").put(memoryTag)
                     .put(']');
         }

---

231-235: Return EINVAL-like errno for invalid unmap arguments.

Use the new errno helper instead of 0.

-        if (address <= 0 || len <= 0) {
-            throw CairoException.critical(0)
+        if (address <= 0 || len <= 0) {
+            throw CairoException.critical(Files.errnoInvalidParameter())
                     .put("unmap: invalid address or length [address=" + address + ", len=" + len + ']');
         }

---

293-304: Unify MAP_FAILED check in mremap accounting.

Mirror mmap path by using FilesFacade.MAP_FAILED.

-        if (address != -1) {
+        if (address != FilesFacade.MAP_FAILED) {
             if (oldMemoryTag == memoryTag) {
                 Unsafe.recordMemAlloc(newSize - previousSize, memoryTag);
             } else {
                 Unsafe.recordMemAlloc(newSize, memoryTag);
                 Unsafe.recordMemAlloc(-previousSize, oldMemoryTag);
             }
         }

---

306-316: Treat munmap success explicitly (result == 0); throw on non‑zero.

This is clearer and avoids accidental success if return contract ever changes.

-        int result = Files.munmap0(address, len);
-        if (result != -1) {
+        int result = Files.munmap0(address, len);
+        if (result == 0) {
             Unsafe.recordMemAlloc(-len, memoryTag);
         } else {
             throw CairoException.critical(Os.errno())
                     .put("munmap failed [address=").put(address)
                     .put(", len=").put(len)
                     .put(", memoryTag=").put(memoryTag).put(']');
         }

---

281-283: Update comment: unmap is executed outside the synchronized section, not “offloaded”.

The current comment suggests a background worker that doesn’t exist here.

-        // offload the unmap to a single thread to not block everyone under synchronized section
+        // perform unmap outside the synchronized section to avoid blocking other threads
         unmap0(unmapPtr, unmapLen, unmapTag);

---

50-61: Optional: validate offset >= 0 for early parameter sanity.

Negative offsets will fail in native anyway; failing early yields clearer errors and avoids syscalls.

---

122-124: Potential accounting mismatch when bypassing cache.

When bypassing cache, mremap0(..., oldMemoryTag=memoryTag, memoryTag) assumes unchanged tag. If callers ever change tags across remaps, memory accounting will be off.

Would you like a follow-up patch to plumb the previous tag (or document the invariant that tags must not change across non-cached remaps)?

core/src/main/java/io/questdb/cairo/wal/CheckWalTransactionsJob.java (1)

106-109: Optional: log swallowed race for observability

You correctly gate on isFileCannotRead(). Consider a low-level log to aid production triage when this race is frequent.

-                    } catch (CairoException e) {
-                        if (!e.isFileCannotRead()) {
-                            throw e;
-                        } // race, table is dropped, ApplyWal2TableJob is already deleting the files
-                    }
+                    } catch (CairoException e) {
+                        if (!e.isFileCannotRead()) {
+                            throw e;
+                        }
+                        // race, table is dropped, ApplyWal2TableJob is already deleting the files
+                        if (LOG.isEnabled(LogLevel.DEBUG)) {
+                            LOG.debug().$("txn file unreadable during check (likely drop in progress) [table=")
+                                    .$(tableToken).I$();
+                        }
+                    }

core/src/main/java/io/questdb/cairo/wal/WalPurgeJob.java (1)

427-433: Minor: capture errno once to keep logs consistent

Avoid calling ff.errno() twice; capture once after the failing call so the message reflects the same errno used in the condition.

-        if (!ff.rmdir(path, false) && !Files.isErrnoFileDoesNotExist(ff.errno())) {
-            LOG.debug()
-                    .$("could not delete directory [path=").$(path)
-                    .$(", errno=").$(ff.errno())
-                    .I$();
+        final boolean removed = ff.rmdir(path, false);
+        final int errno = removed ? 0 : ff.errno();
+        if (!removed && !Files.isErrnoFileDoesNotExist(errno)) {
+            LOG.debug()
+                    .$("could not delete directory [path=").$(path)
+                    .$(", errno=").$(errno)
+                    .I$();
             return false;
         }

core/src/main/java/io/questdb/cairo/wal/seq/TableTransactionLogV1.java (2)

372-379: Use ff.errno() for consistency with FilesFacade-backed calls

For map failures, prefer ff.errno() (already used elsewhere) to keep errno sourcing consistent with the facade.

-                if (newAddr == FilesFacade.MAP_FAILED) {
-                    throw CairoException.critical(Os.errno()).put("cannot mmap transaction log [path=").put(path).put(']');
+                if (newAddr == FilesFacade.MAP_FAILED) {
+                    throw CairoException.critical(ff.errno()).put("cannot mmap transaction log [path=").put(path).put(']');
                 }

---

391-395: Same here: prefer ff.errno() on remap failure

-            if (newAddr == FilesFacade.MAP_FAILED) {
-                throw CairoException.critical(Os.errno()).put("cannot remap transaction log [fd=").put(fd).put(']');
+            if (newAddr == FilesFacade.MAP_FAILED) {
+                throw CairoException.critical(ff.errno()).put("cannot remap transaction log [fd=").put(fd).put(']');
             }

core/src/main/java/io/questdb/cairo/wal/WalWriter.java (1)

1759-1766: Minor nit: avoid repeating the async check

Cache the async flag once to reduce duplication.

-                if (commitMode != CommitMode.NOSYNC) {
-                    ff.msync(auxMemAddr, auxMemSize, commitMode == CommitMode.ASYNC);
-                }
+                final boolean async = commitMode == CommitMode.ASYNC;
+                if (commitMode != CommitMode.NOSYNC) {
+                    ff.msync(auxMemAddr, auxMemSize, async);
+                }
...
-                if (commitMode != CommitMode.NOSYNC) {
-                    ff.msync(dataMemAddr, varColSize, commitMode == CommitMode.ASYNC);
-                }
+                final boolean async = commitMode == CommitMode.ASYNC;
+                if (commitMode != CommitMode.NOSYNC) {
+                    ff.msync(dataMemAddr, varColSize, async);
+                }

Also applies to: 1776-1783

core/src/main/java/io/questdb/cairo/TableWriter.java (1)

144-146: Remove redundant static imports or the wildcard to avoid duplication.

You have both import static io.questdb.cairo.TableUtils.*; and explicit openAppend/openRO static imports. Prefer one style; given the wildcard, the two explicit imports are redundant.

 import static io.questdb.cairo.TableUtils.*;
-import static io.questdb.cairo.TableUtils.openAppend;
-import static io.questdb.cairo.TableUtils.openRO;

core/src/main/java/io/questdb/cairo/wal/seq/TableTransactionLog.java (1)

343-360: Unify errno source and enrich context in mmap failure path

Prefer ff.errno() (used elsewhere in this PR) over Os.errno() and include txnMetaOffset in the error to aid diagnosis.

-                            } else {
-                                close();
-                                throw CairoException.critical(Os.errno())
-                                        .put("cannot mmap table transaction log [path=").put(path)
-                                        .put(", txnMetaOffsetHi=").put(txnMetaOffsetHi)
-                                        .put(']');
+                            } else {
+                                close();
+                                throw CairoException.critical(ff.errno())
+                                        .put("cannot mmap table transaction log [path=").put(path)
+                                        .put(", txnMetaOffsetLo=").put(txnMetaOffset)
+                                        .put(", txnMetaOffsetHi=").put(txnMetaOffsetHi)
+                                        .put(']');

core/src/main/java/io/questdb/cairo/wal/CopyWalSegmentUtils.java (2)

158-163: Guard fsync on optional FDs

srcVarFd/dstVarFd can be -1 for fixed-size targets. Guard to avoid unnecessary syscalls/errors.

-                if (commitMode != CommitMode.NOSYNC) {
-                    ff.fsync(srcFixFd);
-                    ff.fsync(srcVarFd);
-                    ff.fsync(dstFixFd);
-                    ff.fsync(dstVarFd);
-                }
+                if (commitMode != CommitMode.NOSYNC) {
+                    ff.fsync(srcFixFd);
+                    if (srcVarFd > -1) ff.fsync(srcVarFd);
+                    ff.fsync(dstFixFd);
+                    if (dstVarFd > -1) ff.fsync(dstVarFd);
+                }

---

224-231: Defensive unmap check

Given munmap now fails on address == 0, add a guard to avoid spurious failures if the returned address ever equals 0.

-        try {
+        try {
             Vect.flattenIndex(srcDataTimestampAddr, rowCount);
             if (commitMode != CommitMode.NOSYNC) {
                 ff.msync(srcDataTimestampAddr, size, commitMode == CommitMode.ASYNC);
             }
         } finally {
-            ff.munmap(srcDataTimestampAddr, size, MEMORY_TAG);
+            if (srcDataTimestampAddr > 0) {
+                ff.munmap(srcDataTimestampAddr, size, MEMORY_TAG);
+            }
         }

core/src/main/java/io/questdb/cairo/TableUtils.java (1)

1108-1110: Add runtime guard before munmap

An assert may be disabled in production. Since munmap(0, ...) now fails, add a guard to avoid false-positive failures.

-        long adjustedAddress = address - alignedExtraLen;
-        assert adjustedAddress > 0 : "address <= alignedExtraLen";
-        ff.munmap(adjustedAddress, size + alignedExtraLen, memoryTag);
+        long adjustedAddress = address - alignedExtraLen;
+        assert adjustedAddress > 0 : "address <= alignedExtraLen";
+        if (adjustedAddress > 0) {
+            ff.munmap(adjustedAddress, size + alignedExtraLen, memoryTag);
+        }

core/src/main/java/io/questdb/std/Files.java (1)

287-295: Include POSIX EACCES in isErrnoFileCannotRead()
No existing POSIX “permission denied” constant—define, for example,

// in CairoException
public static final int ERRNO_ACCESS_DENIED_POSIX = 13;

then update Files.isErrnoFileCannotRead() to also check

|| errno == CairoException.ERRNO_ACCESS_DENIED_POSIX

core/src/test/java/io/questdb/test/FilesTest.java (3)

768-793: Avoid masking the root failure by calling munmap with len=0 in the success path.

If mmap erroneously succeeds, the subsequent munmap(..., 0, ...) throws a different error and can obscure the diagnosis. Let Assert.fail fire immediately after mmap instead.

-                    long mmapAddr = Files.mmap(fdro, 0, 0, Files.MAP_RO, MemoryTag.MMAP_DEFAULT);
-                    Files.munmap(mmapAddr, 0, MemoryTag.MMAP_DEFAULT);
+                    long mmapAddr = Files.mmap(fdro, 0, 0, Files.MAP_RO, MemoryTag.MMAP_DEFAULT);
                     Assert.fail("mmap with zero len should have failed");

---

795-827: Don’t overwrite the original mapping and drop the zero-len munmap.

Overwriting mmapAddr complicates cleanup; the zero-length munmap similarly hides the intended failure. Keep the original address for cleanup and fail immediately if mremap doesn’t throw.

-                        mmapAddr = Files.mremap(fdro, mmapAddr, 64, 0, 0, Files.MAP_RO, MemoryTag.MMAP_DEFAULT);
-                        Files.munmap(mmapAddr, 0, MemoryTag.MMAP_DEFAULT);
+                        long ignored = Files.mremap(fdro, mmapAddr, 64, 0, 0, Files.MAP_RO, MemoryTag.MMAP_DEFAULT);
                         Assert.fail("mremap with len 0 should have failed");

---

869-885: Assert FDs are valid after openCleanRW.

Minor robustness: assert the returned FDs are positive before using/closing.

-                long fd = Files.openCleanRW(path.$(), 1024);
+                long fd = Files.openCleanRW(path.$(), 1024);
+                Assert.assertTrue(fd > 0);
                 Assert.assertTrue(Files.exists(path.$()));
                 Assert.assertEquals(1024, Files.length(path.$()));
 
-                long fd2 = Files.openCleanRW(path.$(), 2048);
+                long fd2 = Files.openCleanRW(path.$(), 2048);
+                Assert.assertTrue(fd2 > 0);
                 Assert.assertEquals(2048, Files.length(path.$()));

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.