diff options
author | Mårten Nordheim <marten.nordheim@qt.io> | 2024-04-10 18:40:14 +0200 |
---|---|---|
committer | Mårten Nordheim <marten.nordheim@qt.io> | 2024-04-12 14:59:40 +0200 |
commit | 9d8e233284aa9cd0757e1181451f4220473c71fe (patch) | |
tree | 719cadbfbbf74a8ee13f11a516e9218f058686df | |
parent | efd6786e52639ef1ceab819265b0c33d7ef8294e (diff) |
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
Pick-to: 6.7
Change-Id: Id3c8e0b5703da280c84e466ed88287e99788d077
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
-rw-r--r-- | src/corelib/io/qfsfileengine_p.h | 3 | ||||
-rw-r--r-- | src/corelib/io/qfsfileengine_win.cpp | 32 | ||||
-rw-r--r-- | src/corelib/io/qtemporaryfile.cpp | 9 |
3 files changed, 43 insertions, 1 deletions
diff --git a/src/corelib/io/qfsfileengine_p.h b/src/corelib/io/qfsfileengine_p.h index 3b148e1dbb..98c9463c9d 100644 --- a/src/corelib/io/qfsfileengine_p.h +++ b/src/corelib/io/qfsfileengine_p.h @@ -151,6 +151,9 @@ public: #ifndef Q_OS_WIN bool isSequentialFdFh() const; #endif +#ifdef Q_OS_WIN + bool nativeRenameOverwrite(const QString &newName) 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 7a4f99cc7b..3685a3cde5 100644 --- a/src/corelib/io/qfsfileengine_win.cpp +++ b/src/corelib/io/qfsfileengine_win.cpp @@ -27,6 +27,8 @@ #define SECURITY_WIN32 #include <security.h> +#include <memory> + #ifndef PATH_MAX #define PATH_MAX FILENAME_MAX #endif @@ -394,6 +396,36 @@ bool QFSFileEnginePrivate::nativeIsSequential() const || (fileType == FILE_TYPE_PIPE); } +bool QFSFileEnginePrivate::nativeRenameOverwrite(const QString &newName) const +{ + if (fileHandle == INVALID_HANDLE_VALUE) + return false; + QFileSystemEntry newEntry(newName, QFileSystemEntry::FromInternalPath()); + const QString newFilePath = newEntry.nativeFilePath(); + const size_t nameByteLength = newFilePath.length() * sizeof(wchar_t); + if (nameByteLength + sizeof(wchar_t) > std::numeric_limits<DWORD>::max()) + return false; + + constexpr size_t RenameInfoSize = sizeof(FILE_RENAME_INFO); + const size_t renameDataSize = RenameInfoSize + nameByteLength + sizeof(wchar_t); + QVarLengthArray<char> v(qsizetype(renameDataSize), 0); + + auto *renameInfo = q20::construct_at(reinterpret_cast<FILE_RENAME_INFO *>(v.data())); + auto renameInfoRAII = qScopeGuard([&] { std::destroy_at(renameInfo); }); + 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; +} + bool QFSFileEngine::caseSensitive() const { return false; diff --git a/src/corelib/io/qtemporaryfile.cpp b/src/corelib/io/qtemporaryfile.cpp index a99ad22223..59f52bd082 100644 --- a/src/corelib/io/qtemporaryfile.cpp +++ b/src/corelib/io/qtemporaryfile.cpp @@ -184,8 +184,9 @@ static bool createFileFromTemplate(NativeFileHandle &file, QTemporaryFileName &t const DWORD shareMode = (flags & QTemporaryFileEngine::Win32NonShared) ? 0u : (FILE_SHARE_READ | FILE_SHARE_WRITE); + 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); @@ -390,6 +391,12 @@ bool QTemporaryFileEngine::renameOverwrite(const QString &newName) QFSFileEngine::close(); return ok; } +#ifdef Q_OS_WIN + if (d_func()->nativeRenameOverwrite(newName)) { + QFSFileEngine::close(); + return true; + } +#endif QFSFileEngine::close(); return QFSFileEngine::renameOverwrite(newName); } |