From 92f2e0309e05e2aa98c0381b90814a4c4284c122 Mon Sep 17 00:00:00 2001 From: leonarski_f Date: Sun, 23 Aug 2026 12:44:04 +0200 Subject: [PATCH] Keep the raw file alive while it is read without the lock GetRawImage takes the chunk address under hdf5_mutex, drops the lock, and then reads through a borrowed RawFile*. ReadFile() and Close() both take that same lock and call Clear(), which empties the dataset cache and closes the descriptor - so a read racing a close read through a freed object and a recycled fd. Not reachable today, since the only callers of GetRawImage are the rugnux workers and jfjoch_extract_hkl and neither closes concurrently, but the whole point of the change is that the read happens outside the lock. Share the RawFile rather than borrowing it, so the descriptor outlives a Clear() that races it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z --- reader/HDF5ImageSource.cpp | 4 ++-- reader/HDF5ImageSource.h | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/reader/HDF5ImageSource.cpp b/reader/HDF5ImageSource.cpp index 6022ed90..c207ccfa 100644 --- a/reader/HDF5ImageSource.cpp +++ b/reader/HDF5ImageSource.cpp @@ -116,7 +116,7 @@ HDF5ImageSource::GetDataset(const HDF5ImageLocator::Location &loc) const { entry.algorithm = dcpl.GetCompression(); if (entry.direct_chunk && !loc.path.empty()) { - entry.raw = std::make_unique(loc.path); + entry.raw = std::make_shared(loc.path); if (!entry.raw->IsOpen()) entry.raw.reset(); hid_t fcpl = H5Fget_create_plist(loc.file->GetID()); @@ -148,7 +148,7 @@ HDF5ImageSource::PrepareDirectRead(const HDF5ImageLocator::Location &loc) const if (address == HADDR_UNDEF || size == 0) return {}; - return DirectChunk{ds.raw.get(), ds.user_block + address, static_cast(size), + return DirectChunk{ds.raw, ds.user_block + address, static_cast(size), ds.width, ds.height, ds.mode, ds.algorithm}; } diff --git a/reader/HDF5ImageSource.h b/reader/HDF5ImageSource.h index f2763152..97cdaf30 100644 --- a/reader/HDF5ImageSource.h +++ b/reader/HDF5ImageSource.h @@ -47,7 +47,10 @@ public: // Where the bytes of one image are, and what they decode to. Everything needed to read an image // without calling HDF5 again. struct DirectChunk { - const RawFile *file = nullptr; + // Shared, not borrowed: GetRawImage drops the HDF5 lock before reading through this, so a + // concurrent Clear() - which ReadFile() and Close() both do - would otherwise free the file + // and close its descriptor under the reader. + std::shared_ptr file; uint64_t address = 0; uint32_t size = 0; hsize_t width = 0; @@ -100,7 +103,7 @@ private: struct OpenDataset { std::shared_ptr file; std::unique_ptr dataset; - std::unique_ptr raw; + std::shared_ptr raw; // HDF5 addresses count from the end of the user block, so they are file offsets only once // its size is added. Zero for everything this project writes, but not for every file. uint64_t user_block = 0;