Skip to content

Commit

Permalink
Fix network files hanging while the network disconnected (part 3)
Browse files Browse the repository at this point in the history
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: notepad-plus-plus#15669

Improve notepad-plus-plus#4306, notepad-plus-plus#6178, notepad-plus-plus#8055, notepad-plus-plus#11388, notepad-plus-plus#12553, notepad-plus-plus#15540
Close notepad-plus-plus#15679
  • Loading branch information
donho committed Oct 8, 2024
1 parent fb6d79b commit 1445487
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 3 deletions.
57 changes: 57 additions & 0 deletions PowerEditor/src/MISC/Common/Common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<CreateFileParamResult*>(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;
}
1 change: 1 addition & 0 deletions PowerEditor/src/MISC/Common/Common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
6 changes: 3 additions & 3 deletions PowerEditor/src/MISC/Common/FileInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -52,15 +52,15 @@ 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
// If file did exist while calling PathFileExistsW, but before calling CreateFileW, file is deleted: use TRUNCATE_EXISTING will cause the error
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))
Expand Down

0 comments on commit 1445487

Please sign in to comment.