Skip to content

Commit 671f9c5

Browse files
bvu405meta-codesync[bot]
authored andcommitted
Fix SnapshotReader temp-file collision when concurrent readers open the same compressed snapshot
Summary: For compressed `.pytb` inputs, `SnapshotReader::open()` decompresses into a temp file and mmaps it `MAP_SHARED` with `PROT_READ | PROT_WRITE` so the rest of the reader can resolve offsets through `data_`. The temp path was deterministic — `<path>.decompressed.tmp` — which means two `SnapshotReader::open(path)` calls against the same compressed file (commonly a Python process plus a sibling `tintype_debug_launcher` subprocess attached via dapper) collide on that path. The second open is `O_RDWR | O_CREAT | O_TRUNC` followed by `ftruncate(decompressedSize)`, which through the shared inode zero-fills the first reader's mmap. `getManifest()` then returns `decompressedSize` bytes of `\0` until the second `ZSTD_decompress` finishes repopulating the file (~tens of milliseconds). In our case `extract_revision()` saw the null buffer, raised `json.JSONDecodeError: Expecting value: line 1 column 1 (char 0)`, and the deepdump python postmortem bootstrapper aborted before the agent ever ran. This diff switches the compressed-input path to `mkstemp(<path>.decompressed.XXXXXX)`, mirroring the pattern already used by `SnapshotWriter::open` (`mkstemp("/tmp/snapshot_XXXXXX")`). Each reader gets its own temp file, so concurrent opens cannot race on the same inode. `close()` already unlinks `tempFilePath_`, so cleanup is unchanged. The mode goes from `0644` to `0600` (mkstemp default), which is stricter and consistent with the writer. Adds `<cstdlib>` for `mkstemp` and `<vector>` for the writable template buffer. Reviewed By: aperez Differential Revision: D103755545 fbshipit-source-id: 64e44529801ac82d986c764ed119c6f8786b37a1
1 parent 7e0f7bd commit 671f9c5

1 file changed

Lines changed: 14 additions & 4 deletions

File tree

snapshot_lib/SnapshotReader.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515

1616
#include <algorithm>
1717
#include <cerrno>
18+
#include <cstdlib>
1819
#include <cstring>
1920
#include <fstream>
2021
#include <stdexcept>
22+
#include <vector>
2123

2224
namespace facebook::tintype::snapshot {
2325

@@ -148,14 +150,22 @@ bool SnapshotReader::open(const std::string& path) {
148150
return false;
149151
}
150152

151-
// Create a temporary file for the decompressed data
152-
tempFilePath_ = path + ".decompressed.tmp";
153-
tempFd_ = ::open(tempFilePath_.c_str(), O_RDWR | O_CREAT | O_TRUNC, 0644);
153+
// Create a unique temporary file for the decompressed data.
154+
// mkstemp guarantees a fresh path so concurrent readers of the same
155+
// compressed source file (e.g., a sibling process attaching
156+
// tintype_debug_launcher to the same .pytb) cannot collide on this temp
157+
// file. With a shared name, one reader's O_TRUNC + ftruncate corrupts
158+
// another reader's MAP_SHARED mmap (zero-fills the buffer through the
159+
// shared inode) — see SnapshotWriter::open which already uses mkstemp
160+
// for the same reason.
161+
std::string tempTemplate = path + ".decompressed.XXXXXX";
162+
tempFd_ = ::mkstemp(tempTemplate.data());
154163
if (tempFd_ < 0) {
155-
lastError_ = "Failed to create temporary file: " + tempFilePath_ +
164+
lastError_ = "Failed to create temporary file: " + tempTemplate +
156165
" (errno: " + std::to_string(errno) + ")";
157166
return false;
158167
}
168+
tempFilePath_ = std::move(tempTemplate);
159169

160170
// Extend the file to the decompressed size
161171
if (ftruncate(tempFd_, decompressedSize) < 0) {

0 commit comments

Comments
 (0)