Skip to content

Commit f1de798

Browse files
Fix data race in decompressed-assembly cache writer
The background writer thread read directly from the shared uncompressed_assemblies_data_buffer, but on a cache miss that same buffer is handed to the runtime once the decompress lock is released, and the runtime may write into the assembly image (the reason the cache-hit path maps the file MAP_PRIVATE / COW). Concurrent writes could persist a torn or post-mutation image; since the staleness footer only hashes the *compressed* payload, that corrupt image would then be reloaded from cache as if pristine on the next launch. Take a private snapshot of the decompressed bytes in enqueue_write, while the caller still holds assembly_decompress_mutex and before the buffer is exposed to the runtime, so the writer only ever touches immutable memory it owns. On allocation failure we skip caching that assembly rather than aborting. Trade-off: this adds one memcpy per newly-cached assembly on the first-launch (cache-miss) path and holds the queued snapshots (up to the touched working set) transiently until the writer drains them. Subsequent launches hit the mmap path and never enqueue. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9ba5340 commit f1de798

1 file changed

Lines changed: 27 additions & 9 deletions

File tree

src/native/clr/host/assembly-store.cc

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
#include <cerrno>
22
#include <cstdlib>
3+
#include <cstring>
34
#include <condition_variable>
45
#include <deque>
6+
#include <memory>
57
#include <string>
68
#include <thread>
79

@@ -48,10 +50,15 @@ namespace {
4850

4951
struct WriteRequest final
5052
{
51-
std::string path;
52-
const uint8_t *data; // stable pointer into uncompressed_assemblies_data_buffer
53-
size_t size;
54-
uint64_t token;
53+
std::string path;
54+
// Private snapshot of the decompressed bytes, taken at enqueue time
55+
// while the shared decompression buffer is still pristine. Owning a
56+
// copy (rather than pointing into uncompressed_assemblies_data_buffer)
57+
// avoids racing with the runtime, which may write into the image it
58+
// receives once the decompress lock is released.
59+
std::unique_ptr<uint8_t[]> data;
60+
size_t size;
61+
uint64_t token;
5562
};
5663

5764
std::mutex state_lock;
@@ -90,7 +97,7 @@ namespace {
9097
bool ok = true;
9198
size_t off = 0;
9299
while (off < req.size) {
93-
ssize_t written = write (fd, req.data + off, req.size - off);
100+
ssize_t written = write (fd, req.data.get () + off, req.size - off);
94101
if (written < 0) {
95102
if (errno == EINTR) {
96103
continue;
@@ -212,19 +219,30 @@ namespace {
212219
return static_cast<uint8_t*>(mapped);
213220
}
214221

215-
// Queues a freshly decompressed assembly to be persisted. The data
216-
// pointer must remain valid and immutable for the process lifetime
217-
// (it points into uncompressed_assemblies_data_buffer).
222+
// Queues a freshly decompressed assembly to be persisted. Takes a private
223+
// snapshot of the bytes up front: the caller holds assembly_decompress_mutex
224+
// and has not yet handed the shared buffer to the runtime, so the data is
225+
// still pristine here. Copying now (rather than letting the background
226+
// thread read the shared buffer later) avoids racing with the runtime,
227+
// which may write into the image once the lock is released.
218228
// Must be called while holding assembly_decompress_mutex.
219229
void enqueue_write (std::string_view name, const uint8_t *data, size_t size, uint64_t token) noexcept
220230
{
221231
if (!enabled) {
222232
return;
223233
}
224234

235+
auto snapshot = std::unique_ptr<uint8_t[]> (new (std::nothrow) uint8_t[size]);
236+
if (snapshot == nullptr) {
237+
// Out of memory: skip caching this assembly. Not fatal — the next
238+
// launch simply decompresses it again.
239+
return;
240+
}
241+
memcpy (snapshot.get (), data, size);
242+
225243
WriteRequest req {
226244
.path = build_path (name),
227-
.data = data,
245+
.data = std::move (snapshot),
228246
.size = size,
229247
.token = token,
230248
};

0 commit comments

Comments
 (0)