Fix concurrent INSTALL/LOAD extension race on cold cache#711
Merged
Conversation
Three TOCTOU races in the extension install path cause SIGBUS/SIGSEGV or unhandled IOException when multiple connections (across threads or processes) INSTALL the same extension on a cold cache: 1. createDir race: LocalFileSystem::createDir checked exists() before calling create_directories(). Two concurrent callers both passed the check and one hit "Directory already exists". Fixed by removing the TOCTOU pre-check and calling create_directories directly (returns false on "already exists" without setting errCode; we only throw on errCode). 2. File mutation under dlopen: tryDownloadExtensionFile opened the destination with CREATE_AND_TRUNCATE_IF_EXISTS and wrote in-place. If another process had dlopend the same path, the truncation / partial write caused the dynamic loader to fault on mutated mmap content (SIGBUS / SIGSEGV). Fixed by downloading to a unique temp file (thread-id hashed + atomic counter), then rename()ing into place. POSIX rename() is atomic on the same filesystem: readers holding the old inode keep a consistent view, new opens always see the complete file. 3. Wasted download race: Two processes both passed the fileOrPathExists check before the first one had finished writing. Added a re-check after the dir- creation section so the second process skips the download when the first already placed the file. Co-Authored-By: Álvaro Tejero-Cantero <alvorithm@users.noreply.github.com>
adsharma
force-pushed
the
fix/issue-710-extension-install-race
branch
from
July 21, 2026 22:22
e3f8445 to
627d765
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #710. Makes extension installation process-safe on a cold cache by eliminating three TOCTOU races.
Races fixed
1.
createDirraceLocalFileSystem::createDirdid anexists()pre-check before callingcreate_directories(), creating a TOCTOU window. Two concurrent callers both passed the check and one threw "Directory already exists".Fix: Removed the
exists()check.std::filesystem::create_directoriesalready returnsfalse(without settingerrCode) when the directory already exists — we only throw on actual filesystem errors (errCodeset).2. File mutation under
dlopen(SIGBUS/SIGSEGV)tryDownloadExtensionFileopened the destination withCREATE_AND_TRUNCATE_IF_EXISTSand wrote in-place. If another connection haddlopend the same path (across threads or processes), the truncation or partial write caused the dynamic loader to fault on mutated mmap content → SIGBUS (truncated past EOF) or SIGSEGV (hash-walk over partial content).Fix: Download to a unique temp file (
{path}.tmp.{thread_hash}.{counter}), thenrename()into place. POSIXrename()is atomic on the same filesystem:dlopenkeep a consistent view of the old file.Temp files are cleaned up on both write failure and rename failure.
3. Wasted download race
Two processes both passed the
fileOrPathExistscheck before the first finished writing. Added a re-check after the dir-creation section so the second process skips the download.Testing
extension~extension.*)AnyGraphLabelsFunctions)createDirchange passes all existing tests (pre-existing failures incopy_to_csv,copy_to_parquet,copy_to_big_resultsalso fail onmain)