mirror of
https://github.com/official-stockfish/Stockfish.git
synced 2026-07-22 12:47:08 +00:00
Fix partial MADV_POPULATE_WRITE
Potentially fixes https://github.com/official-stockfish/Stockfish/issues/6846 I'm not a Linux guru but here's my understanding of what's going on, from looking at strace logs. If `/dev/shm` is undersized or too full, `MADV_POPULATE_WRITE` can partially fault in the pages and then return an error. The crash then happens in the second process, on the first read to `header_ptr_->initialized`. There's a TOCTOU between when the region is attachable (the second process can see it in `/dev/shm`) and when we can actually safely read from it. The SIGBUS happens because `/dev/shm` is so full from partially faulted pages that we can't even fault in a single additional page. This didn't happen before because a failed `posix_fallocate` *rolls back any faulted pages*. Then the second process faults in one page (which succeeds), reads a `0` from `header_ptr_->initialized`, declares the region invalid, and continues to try (and fail) to set up its own region. There's still a problem here, methinks, if the process gets killed/interrupted between the failed `madvise` and the `ftruncate`. I think a more robust approach is to create the file anonymously first (to prevent other processes from attaching to it), then `linkat` to publish it atomically, but that would be more implementation work. (Could create a follow-up issue for that as an nice first issue for new contribs?) closes https://github.com/official-stockfish/Stockfish/pull/6852 No functional change
This commit is contained in:
committed by
Joost VandeVondele
parent
5b068c96d2
commit
05f48577e1
@@ -601,6 +601,11 @@ class SharedMemory: public detail::SharedMemoryBase {
|
|||||||
// If the THP population failed, try with fallocate
|
// If the THP population failed, try with fallocate
|
||||||
if (!populated && posix_fallocate(fd_, 0, static_cast<off_t>(total_size_)) != 0)
|
if (!populated && posix_fallocate(fd_, 0, static_cast<off_t>(total_size_)) != 0)
|
||||||
{
|
{
|
||||||
|
// Release any partially populated pages by a failed MADV_POPULATE_WRITE.
|
||||||
|
// TODO: Not robust. Need to avoid residual pages if the process
|
||||||
|
// is terminated between the failed madvise and the ftruncate.
|
||||||
|
int err = ftruncate(fd_, 0);
|
||||||
|
(void) err;
|
||||||
munmap(mapped_ptr_, total_size_);
|
munmap(mapped_ptr_, total_size_);
|
||||||
mapped_ptr_ = nullptr;
|
mapped_ptr_ = nullptr;
|
||||||
return false;
|
return false;
|
||||||
|
|||||||
Reference in New Issue
Block a user