From 1445487bb76b92a3a30cf71d8175b8b416b4c0c9 Mon Sep 17 00:00:00 2001 From: Don Ho Date: Sun, 6 Oct 2024 00:43:16 +0200 Subject: [PATCH] Fix network files hanging while the network disconnected (part 3) Add thread for CreateFile to fix saving disconnected network file hanging. STR: Open a network file, modify it. Disconnect the network, then save the file. A huge latency (more than 1 minute) can be observed. Not that the crash is not reproducible anymore by this PR. If any crash happens for you, please let me know (with the STR). Ref: https://github.com/notepad-plus-plus/notepad-plus-plus/pull/15669 Improve #4306, #6178, #8055, #11388, #12553, #15540 Close #15679 --- PowerEditor/src/MISC/Common/Common.cpp | 57 +++++++++++++++++++ PowerEditor/src/MISC/Common/Common.h | 1 + PowerEditor/src/MISC/Common/FileInterface.cpp | 6 +- 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/PowerEditor/src/MISC/Common/Common.cpp b/PowerEditor/src/MISC/Common/Common.cpp index 47154552b..4b86174d5 100644 --- a/PowerEditor/src/MISC/Common/Common.cpp +++ b/PowerEditor/src/MISC/Common/Common.cpp @@ -1945,3 +1945,60 @@ DWORD getFileAttributesExWaitSec(const wchar_t* filePath, WIN32_FILE_ATTRIBUTE_D return data._result; } + + +//---------------------------------------------------- + +struct CreateFileParamResult +{ + wstring _filePath; + HANDLE _hFile = INVALID_HANDLE_VALUE; + DWORD _accessParam = GENERIC_READ | GENERIC_WRITE; + DWORD _shareParam = FILE_SHARE_READ | FILE_SHARE_WRITE; + DWORD _dispParam = CREATE_ALWAYS; + DWORD _attribParam = FILE_ATTRIBUTE_NORMAL; + bool _isNetworkFailure = true; + CreateFileParamResult(wstring filePath, DWORD accessParam, DWORD shareParam, DWORD dispParam, DWORD attribParam) : + _filePath(filePath), _accessParam(accessParam), _shareParam(shareParam), _dispParam(dispParam), _attribParam(attribParam) {}; +}; + +DWORD WINAPI createFileWorker(void* data) +{ + CreateFileParamResult* inAndOut = static_cast(data); + inAndOut->_hFile = ::CreateFileW(inAndOut->_filePath.c_str(), inAndOut->_accessParam, inAndOut->_shareParam, NULL, inAndOut->_dispParam, inAndOut->_attribParam, NULL); + inAndOut->_isNetworkFailure = false; + return ERROR_SUCCESS; +}; + +HANDLE createFileWaitSec(const wchar_t* filePath, DWORD accessParam, DWORD shareParam, DWORD dispParam, DWORD attribParam, DWORD milliSec2wait, bool* isNetWorkProblem) +{ + CreateFileParamResult data(filePath, accessParam, shareParam, dispParam, attribParam); + + HANDLE hThread = ::CreateThread(NULL, 0, createFileWorker, &data, 0, NULL); + if (!hThread) + { + return INVALID_HANDLE_VALUE; + } + + // wait for our worker thread to complete or terminate it when the required timeout has elapsed + DWORD dwWaitStatus = ::WaitForSingleObject(hThread, milliSec2wait == 0 ? DEFAULT_MILLISEC : milliSec2wait); + switch (dwWaitStatus) + { + case WAIT_OBJECT_0: // Ok, the state of our worker thread is signaled, so it finished itself in the timeout given + // - nothing else to do here, except the thread handle closing later + break; + + case WAIT_TIMEOUT: // the timeout interval elapsed, but the worker's state is still non-signaled + default: // Timeout reached, or WAIT_FAILED or WAIT_ABANDONED + // attempt to cancel the operation + ::CancelIoEx(data._hFile, NULL); + ::TerminateThread(hThread, dwWaitStatus); + break; + } + CloseHandle(hThread); + + if (isNetWorkProblem != nullptr) + *isNetWorkProblem = data._isNetworkFailure; + + return data._hFile; +} \ No newline at end of file diff --git a/PowerEditor/src/MISC/Common/Common.h b/PowerEditor/src/MISC/Common/Common.h index b4a8e0b62..4639f9dd3 100644 --- a/PowerEditor/src/MISC/Common/Common.h +++ b/PowerEditor/src/MISC/Common/Common.h @@ -289,3 +289,4 @@ bool doesPathExist(const wchar_t* path, DWORD milliSec2wait = 0, bool* isNetWork DWORD getDiskFreeSpaceWaitSec(const wchar_t* dirPath, ULARGE_INTEGER* freeBytesForUser, DWORD milliSec2wait = 0, bool* isNetWorkProblem = nullptr); DWORD getFileAttributesExWaitSec(const wchar_t* filePath, WIN32_FILE_ATTRIBUTE_DATA* fileAttr, DWORD milliSec2wait = 0, bool* isNetWorkProblem = nullptr); +HANDLE createFileWaitSec(const wchar_t* filePath, DWORD accessParam, DWORD shareParam, DWORD dispParam, DWORD attribParam, DWORD milliSec2wait = 0, bool* isNetWorkProblem = nullptr); diff --git a/PowerEditor/src/MISC/Common/FileInterface.cpp b/PowerEditor/src/MISC/Common/FileInterface.cpp index c2c884fac..73930be72 100644 --- a/PowerEditor/src/MISC/Common/FileInterface.cpp +++ b/PowerEditor/src/MISC/Common/FileInterface.cpp @@ -35,7 +35,7 @@ Win32_IO_File::Win32_IO_File(const wchar_t *fname) bool fileExists = false; // Store the file creation date & attributes for a possible use later... - if (::GetFileAttributesEx(fname, GetFileExInfoStandard, &attributes_original)) // No thread (GetFileAttributesExWaitSec) to prevent eventual crash + if (getFileAttributesExWaitSec(fname, &attributes_original)) { fileExists = (attributes_original.dwFileAttributes != INVALID_FILE_ATTRIBUTES); } @@ -52,7 +52,7 @@ Win32_IO_File::Win32_IO_File(const wchar_t *fname) } } - _hFile = ::CreateFileW(fname, _accessParam, _shareParam, NULL, dispParam, _attribParam, NULL); // No thread (CreateFileWaitSec) due to the lock guard in the caller which leads crash + _hFile = ::createFileWaitSec(fname, _accessParam, _shareParam, dispParam, _attribParam); // Race condition management: // If file didn't exist while calling PathFileExistsW, but before calling CreateFileW, file is created: use CREATE_ALWAYS is OK @@ -60,7 +60,7 @@ Win32_IO_File::Win32_IO_File(const wchar_t *fname) if (dispParam == TRUNCATE_EXISTING && _hFile == INVALID_HANDLE_VALUE && ::GetLastError() == ERROR_FILE_NOT_FOUND) { dispParam = CREATE_ALWAYS; - _hFile = ::CreateFileW(fname, _accessParam, _shareParam, NULL, dispParam, _attribParam, NULL); + _hFile = ::createFileWaitSec(fname, _accessParam, _shareParam, dispParam, _attribParam); } if (fileExists && (dispParam == CREATE_ALWAYS) && (_hFile != INVALID_HANDLE_VALUE))