From 7a32c4aaf3fddcc2617bb60a2a3eb77061b94283 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Wed, 10 Apr 2024 18:40:14 +0200 Subject: [PATCH] QSaveFile[win]: Use SetFileInformationByHandle for atomic rename With our previous setup we would CloseHandle the file handle and then MoveFileEx the file to the final destination. This opens up the opportunity for other processes to open the file in the meantime and cause the MoveFileEx to fail. With SetFileInformationByHandle we can, without closing the handle, rename the file to the final destination. Fixes: QTBUG-122208 Change-Id: Id3c8e0b5703da280c84e466ed88287e99788d077 Reviewed-by: Thiago Macieira (cherry picked from commit 9d8e233284aa9cd0757e1181451f4220473c71fe) --- src/corelib/io/qfsfileengine_p.h | 3 +++ src/corelib/io/qfsfileengine_win.cpp | 36 ++++++++++++++++++++++++++++ src/corelib/io/qtemporaryfile.cpp | 12 +++++++++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/corelib/io/qfsfileengine_p.h b/src/corelib/io/qfsfileengine_p.h index a3d3f80f2ba..646bd149b47 100644 --- a/src/corelib/io/qfsfileengine_p.h +++ b/src/corelib/io/qfsfileengine_p.h @@ -179,6 +179,9 @@ public: #ifndef Q_OS_WIN bool isSequentialFdFh() const; #endif +#if defined(Q_OS_WIN) && !defined(Q_OS_WINRT) + bool nativeRenameOverwrite(const QFileSystemEntry &newEntry) const; +#endif uchar *map(qint64 offset, qint64 size, QFile::MemoryMapFlags flags); bool unmap(uchar *ptr); diff --git a/src/corelib/io/qfsfileengine_win.cpp b/src/corelib/io/qfsfileengine_win.cpp index b1a1ff7112d..3edb2d295ff 100644 --- a/src/corelib/io/qfsfileengine_win.cpp +++ b/src/corelib/io/qfsfileengine_win.cpp @@ -49,6 +49,8 @@ #include "qdatetime.h" #include "qt_windows.h" +#include "qscopeguard.h" + #include #include #include @@ -66,6 +68,8 @@ # include #endif +#include + #ifndef PATH_MAX #define PATH_MAX FILENAME_MAX #endif @@ -443,6 +447,38 @@ bool QFSFileEnginePrivate::nativeIsSequential() const #endif } +#ifndef Q_OS_WINRT +bool QFSFileEnginePrivate::nativeRenameOverwrite(const QFileSystemEntry &newEntry) const +{ + if (fileHandle == INVALID_HANDLE_VALUE) + return false; + const QString newFilePath = newEntry.nativeFilePath(); + const size_t nameByteLength = newFilePath.length() * sizeof(wchar_t); + if (nameByteLength + sizeof(wchar_t) > std::numeric_limits::max()) + return false; + + constexpr size_t RenameInfoSize = sizeof(FILE_RENAME_INFO); + const size_t renameDataSize = RenameInfoSize + nameByteLength + sizeof(wchar_t); + QVarLengthArray v = QVarLengthArray(int(renameDataSize)); + std::fill_n(v.data(), v.size(), 0); + + auto *renameInfo = new (v.data()) FILE_RENAME_INFO; + auto renameInfoRAII = qScopeGuard([&] { renameInfo->~FILE_RENAME_INFO(); }); + renameInfo->ReplaceIfExists = TRUE; + renameInfo->RootDirectory = nullptr; + renameInfo->FileNameLength = DWORD(nameByteLength); + memcpy(renameInfo->FileName, newFilePath.data(), nameByteLength); + + bool res = SetFileInformationByHandle(fileHandle, FileRenameInfo, renameInfo, + DWORD(renameDataSize)); +#if 0 + if (!res) + qErrnoWarning("QFSFileEnginePrivate::nativeRenameOverwrite failed"); +#endif + return res; +} +#endif // !Q_OS_WINRT + bool QFSFileEngine::caseSensitive() const { return false; diff --git a/src/corelib/io/qtemporaryfile.cpp b/src/corelib/io/qtemporaryfile.cpp index 61206c84536..28605183c20 100644 --- a/src/corelib/io/qtemporaryfile.cpp +++ b/src/corelib/io/qtemporaryfile.cpp @@ -220,8 +220,9 @@ static bool createFileFromTemplate(NativeFileHandle &file, QTemporaryFileName &t ? 0u : (FILE_SHARE_READ | FILE_SHARE_WRITE); # ifndef Q_OS_WINRT + const DWORD extraAccessFlags = (flags & QTemporaryFileEngine::Win32NonShared) ? DELETE : 0; file = CreateFile((const wchar_t *)path.constData(), - GENERIC_READ | GENERIC_WRITE, + GENERIC_READ | GENERIC_WRITE | extraAccessFlags, shareMode, NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL); # else // !Q_OS_WINRT @@ -431,6 +432,15 @@ bool QTemporaryFileEngine::renameOverwrite(const QString &newName) QFSFileEngine::close(); return ok; } +#if defined(Q_OS_WIN) && !defined(Q_OS_WINRT) + if (flags & Win32NonShared) { + QFileSystemEntry newEntry(newName, QFileSystemEntry::FromInternalPath()); + if (d_func()->nativeRenameOverwrite(newEntry)) { + QFSFileEngine::close(); + return true; + } + } +#endif QFSFileEngine::close(); return QFSFileEngine::renameOverwrite(newName); } -- 2.44.0.windows.1