Skip to content

Commit 7ec078e

Browse files
authored
[Support] Avoid a VirtualBox shared folders mmap bug (#78597)
In acd8791, a call to FlushFileBuffers was added to work around a rare kernel bug. In 3b9b4d2, the scope of that workaround was limited, for performance reasons, as the flushes are quite expensive. On VirtualBox shared folders, closing a memory mapping that has been written to, also needs to be explicitly flushed, if renaming the output file before it is closed. Contrary to the kernel bug, this always happens on such mounts. In these cases, the output ends up as a file of the right size, but the contents are all zeros. The sequence to trigger the issue on the VirtualBox Shared Folders is this, summarized: file = CreateFile() mapping = CreateFileMapping(file) mem = MapViewOfFile() CloseHandle(mapping) write(mem) UnmapViewOfFile(mem) SetFileInformationByHandle(file, FileRenameInfo) CloseHandle(file) With this sequence, the output file always ends up with all zeros. See mstorsjo/llvm-mingw#393 for a full reproduction example. To avoid this issue, call FlushFileBuffers() when the file may reside on a VitualBox shared folder. As the flushes are expensive, only do them when the output isn't on a local file system. The issue with VirtualBox shared folders could also be fixed by calling FlushViewOfFile before UnmapViewOfFile, and doing that could be slightly less expensive than FlushFileBuffers. Empirically, the difference between the two is very small though, and as it's not easy to verify whether switching FlushFileBuffers to FlushViewOfFile helps with the rare kernel bug, keep using FlushFileBuffers for both cases, for code simplicity. This fixes downstream bug mstorsjo/llvm-mingw#393.
1 parent 33ec6b3 commit 7ec078e

File tree

1 file changed

+23
-2
lines changed

1 file changed

+23
-2
lines changed

llvm/lib/Support/Windows/Path.inc

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -961,15 +961,36 @@ void mapped_file_region::unmapImpl() {
961961

962962
::UnmapViewOfFile(Mapping);
963963

964-
if (Mode == mapmode::readwrite && Exe && hasFlushBufferKernelBug()) {
964+
if (Mode == mapmode::readwrite) {
965+
bool DoFlush = Exe && hasFlushBufferKernelBug();
965966
// There is a Windows kernel bug, the exact trigger conditions of which
966967
// are not well understood. When triggered, dirty pages are not properly
967968
// flushed and subsequent process's attempts to read a file can return
968969
// invalid data. Calling FlushFileBuffers on the write handle is
969970
// sufficient to ensure that this bug is not triggered.
970971
// The bug only occurs when writing an executable and executing it right
971972
// after, under high I/O pressure.
972-
::FlushFileBuffers(FileHandle);
973+
if (!DoFlush) {
974+
// Separately, on VirtualBox Shared Folder mounts, writes via memory
975+
// maps always end up unflushed (regardless of version of Windows),
976+
// unless flushed with this explicit call, if they are renamed with
977+
// SetFileInformationByHandle(FileRenameInfo) before closing the output
978+
// handle.
979+
//
980+
// As the flushing is quite expensive, use a heuristic to limit the
981+
// cases where we do the flushing. Only do the flushing if we aren't
982+
// sure we are on a local file system.
983+
bool IsLocal = false;
984+
SmallVector<wchar_t, 128> FinalPath;
985+
if (!realPathFromHandle(FileHandle, FinalPath)) {
986+
// Not checking the return value here - if the check fails, assume the
987+
// file isn't local.
988+
is_local_internal(FinalPath, IsLocal);
989+
}
990+
DoFlush = !IsLocal;
991+
}
992+
if (DoFlush)
993+
::FlushFileBuffers(FileHandle);
973994
}
974995

975996
::CloseHandle(FileHandle);

0 commit comments

Comments
 (0)