Skip to content

Commit

Permalink
Fix disconnected network files hanging while saving
Browse files Browse the repository at this point in the history 
This PR prevent hanging when user: Open a network file, modify it. Disconnect the network, then save the file.
It also prevents the zombie process due to blocked CreateFile left behind.

Remove the timeout thread for CreateFile to prevent the zombie process. Use another way for the detection:
If the result of network file existent detection is false, and the network problem found (timeout reached), we just stop and don't call CreateFile routine.

Ref: 1445487

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
  • Loading branch information
@donho
donho committed Oct 12, 2024
1 parent 0ca0348 commit 3b6f22b
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 140 deletions.
134 changes: 16 additions & 118 deletions PowerEditor/src/MISC/Common/Common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1771,70 +1771,6 @@ bool Version::isCompatibleTo(const Version& from, const Version& to) const

#define DEFAULT_MILLISEC 1000

//----------------------------------------------------
struct GetAttrParamResult {
wstring _filePath;
DWORD _fileAttr = INVALID_FILE_ATTRIBUTES;
bool _isNetworkFailure = true;
};

DWORD WINAPI getFileAttributesWorker(void* data)
{
GetAttrParamResult* inAndOut = static_cast<GetAttrParamResult*>(data);
inAndOut->_fileAttr = ::GetFileAttributesW(inAndOut->_filePath.c_str());
inAndOut->_isNetworkFailure = false;
return ERROR_SUCCESS;
};

DWORD getFileAttrWaitSec(const wchar_t* filePath, DWORD milliSec2wait, bool* isNetWorkProblem)
{
GetAttrParamResult data(filePath);

HANDLE hThread = ::CreateThread(NULL, 0, getFileAttributesWorker, &data, 0, NULL);
if (!hThread)
{
return INVALID_FILE_ATTRIBUTES;
}

// 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: // any other dwWaitStatus is a BAD one here
// WAIT_FAILED or WAIT_ABANDONED
::TerminateThread(hThread, dwWaitStatus);
break;
}
CloseHandle(hThread);

if (isNetWorkProblem != nullptr)
*isNetWorkProblem = data._isNetworkFailure;

return data._fileAttr;
};

bool doesFileExist(const wchar_t* filePath, DWORD milliSec2wait, bool* isNetWorkProblem)
{
DWORD attr = getFileAttrWaitSec(filePath, milliSec2wait, isNetWorkProblem);
return (attr != INVALID_FILE_ATTRIBUTES && !(attr & FILE_ATTRIBUTE_DIRECTORY));
}

bool doesDirectoryExist(const wchar_t* dirPath, DWORD milliSec2wait, bool* isNetWorkProblem)
{
DWORD attr = getFileAttrWaitSec(dirPath, milliSec2wait, isNetWorkProblem);
return (attr != INVALID_FILE_ATTRIBUTES && (attr & FILE_ATTRIBUTE_DIRECTORY));
}

bool doesPathExist(const wchar_t* path, DWORD milliSec2wait, bool* isNetWorkProblem)
{
DWORD attr = getFileAttrWaitSec(path, milliSec2wait, isNetWorkProblem);
return (attr != INVALID_FILE_ATTRIBUTES);
}

//----------------------------------------------------

Expand All @@ -1855,7 +1791,7 @@ DWORD WINAPI getDiskFreeSpaceExWorker(void* data)
return ERROR_SUCCESS;
};

DWORD getDiskFreeSpaceWaitSec(const wchar_t* dirPath, ULARGE_INTEGER* freeBytesForUser, DWORD milliSec2wait, bool* isNetWorkProblem)
DWORD getDiskFreeSpaceWithTimeout(const wchar_t* dirPath, ULARGE_INTEGER* freeBytesForUser, DWORD milliSec2wait, bool* isNetWorkProblem)
{
GetDiskFreeSpaceParamResult data(dirPath);

Expand Down Expand Up @@ -1911,7 +1847,7 @@ DWORD WINAPI getFileAttributesExWorker(void* data)
return ERROR_SUCCESS;
};

DWORD getFileAttributesExWaitSec(const wchar_t* filePath, WIN32_FILE_ATTRIBUTE_DATA* fileAttr, DWORD milliSec2wait, bool* isNetWorkProblem)
DWORD getFileAttributesExWithTimeout(const wchar_t* filePath, WIN32_FILE_ATTRIBUTE_DATA* fileAttr, DWORD milliSec2wait, bool* isNetWorkProblem)
{
GetAttrExParamResult data(filePath);

Expand Down Expand Up @@ -1945,61 +1881,23 @@ DWORD getFileAttributesExWaitSec(const wchar_t* filePath, WIN32_FILE_ATTRIBUTE_D
return data._result;
}


//----------------------------------------------------

struct CreateFileParamResult
bool doesFileExist(const wchar_t* filePath, DWORD milliSec2wait, bool* isNetWorkProblem)
{
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) {};
};
WIN32_FILE_ATTRIBUTE_DATA attributes{};
getFileAttributesExWithTimeout(filePath, &attributes, milliSec2wait, isNetWorkProblem);
return (attributes.dwFileAttributes != INVALID_FILE_ATTRIBUTES && !(attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY));
}

DWORD WINAPI createFileWorker(void* data)
bool doesDirectoryExist(const wchar_t* dirPath, DWORD milliSec2wait, bool* isNetWorkProblem)
{
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;
};
WIN32_FILE_ATTRIBUTE_DATA attributes{};
getFileAttributesExWithTimeout(dirPath, &attributes, milliSec2wait, isNetWorkProblem);
return (attributes.dwFileAttributes != INVALID_FILE_ATTRIBUTES && (attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY));
}

HANDLE createFileWaitSec(const wchar_t* filePath, DWORD accessParam, DWORD shareParam, DWORD dispParam, DWORD attribParam, DWORD milliSec2wait, bool* isNetWorkProblem)
bool doesPathExist(const wchar_t* path, 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;
WIN32_FILE_ATTRIBUTE_DATA attributes{};
getFileAttributesExWithTimeout(path, &attributes, milliSec2wait, isNetWorkProblem);
return (attributes.dwFileAttributes != INVALID_FILE_ATTRIBUTES);
}


11 changes: 5 additions & 6 deletions PowerEditor/src/MISC/Common/Common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,10 @@ class Version final
unsigned long _build = 0;
};

DWORD getFileAttrWaitSec(const wchar_t* filePath, DWORD milliSec2wait = 0, bool* isNetWorkProblem = nullptr);

DWORD getDiskFreeSpaceWithTimeout(const wchar_t* dirPath, ULARGE_INTEGER* freeBytesForUser, DWORD milliSec2wait = 0, bool* isNetWorkProblem = nullptr);
DWORD getFileAttributesExWithTimeout(const wchar_t* filePath, WIN32_FILE_ATTRIBUTE_DATA* fileAttr, DWORD milliSec2wait = 0, bool* isNetWorkProblem = nullptr);

bool doesFileExist(const wchar_t* filePath, DWORD milliSec2wait = 0, bool* isNetWorkProblem = nullptr);
bool doesDirectoryExist(const wchar_t* dirPath, DWORD milliSec2wait = 0, bool* isNetWorkProblem = nullptr);
bool doesPathExist(const wchar_t* path, DWORD milliSec2wait = 0, bool* isNetWorkProblem = nullptr);

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);
bool doesPathExist(const wchar_t* path, DWORD milliSec2wait = 0, bool* isNetWorkProblem = nullptr);
18 changes: 12 additions & 6 deletions PowerEditor/src/MISC/Common/FileInterface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ Win32_IO_File::Win32_IO_File(const wchar_t *fname)
WIN32_FILE_ATTRIBUTE_DATA attributes_original{};
DWORD dispParam = CREATE_ALWAYS;
bool fileExists = false;

bool hasNetworkProblem = false;
// Store the file creation date & attributes for a possible use later...
if (getFileAttributesExWaitSec(fname, &attributes_original))
if (getFileAttributesExWithTimeout(fname, &attributes_original, 0, &hasNetworkProblem))
{
fileExists = (attributes_original.dwFileAttributes != INVALID_FILE_ATTRIBUTES);
}
Expand All @@ -51,16 +51,22 @@ Win32_IO_File::Win32_IO_File(const wchar_t *fname)
FindClose(hFind);
}
}
else
{
bool isFromNetwork = PathIsNetworkPath(fname);
if (isFromNetwork && hasNetworkProblem) // The file doesn't exist, and the file is a network file, plus the network problem has been detected due to timeout
return; // In this case, we don't call createFile to prevent hanging
}

_hFile = ::createFileWaitSec(fname, _accessParam, _shareParam, dispParam, _attribParam);
_hFile = ::CreateFileW(fname, _accessParam, _shareParam, NULL, dispParam, _attribParam, NULL);

// 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 file didn't exist while calling getFileAttributesExWithTimeout, but before calling CreateFileW, file is created: use CREATE_ALWAYS is OK
// If file did exist while calling getFileAttributesExWithTimeout, 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 = ::createFileWaitSec(fname, _accessParam, _shareParam, dispParam, _attribParam);
_hFile = ::CreateFileW(fname, _accessParam, _shareParam, NULL, dispParam, _attribParam, NULL);
}

if (fileExists && (dispParam == CREATE_ALWAYS) && (_hFile != INVALID_HANDLE_VALUE))
Expand Down
21 changes: 11 additions & 10 deletions PowerEditor/src/ScintillaComponent/Buffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ void Buffer::updateTimeStamp()
{
FILETIME timeStampLive {};
WIN32_FILE_ATTRIBUTE_DATA attributes{};
if (getFileAttributesExWaitSec(_fullPathName.c_str(), &attributes) != FALSE)
if (getFileAttributesExWithTimeout(_fullPathName.c_str(), &attributes) != FALSE)
{
timeStampLive = attributes.ftLastWriteTime;
}
Expand Down Expand Up @@ -311,7 +311,7 @@ bool Buffer::checkFileState() // returns true if the status has been changed (it
isOK = true;
}
}
else if (getFileAttributesExWaitSec(_fullPathName.c_str(), &attributes) != FALSE)
else if (getFileAttributesExWithTimeout(_fullPathName.c_str(), &attributes) != FALSE)
{
int mask = 0; //status always 'changes', even if from modified to modified
bool isFileReadOnly = attributes.dwFileAttributes & FILE_ATTRIBUTE_READONLY;
Expand Down Expand Up @@ -710,7 +710,7 @@ BufferID FileManager::loadFile(const wchar_t* filename, Document doc, int encodi
if (pPath)
{
WIN32_FILE_ATTRIBUTE_DATA attributes{};
if (getFileAttributesExWaitSec(pPath, &attributes) != FALSE)
if (getFileAttributesExWithTimeout(pPath, &attributes) != FALSE)
{
LARGE_INTEGER size{};
size.LowPart = attributes.nFileSizeLow;
Expand Down Expand Up @@ -845,7 +845,7 @@ bool FileManager::reloadBuffer(BufferID id)
//Get file size
int64_t fileSize = 0;
WIN32_FILE_ATTRIBUTE_DATA attributes{};
getFileAttributesExWaitSec(buf->getFullPathName(), &attributes);
getFileAttributesExWithTimeout(buf->getFullPathName(), &attributes);
if (attributes.dwFileAttributes == INVALID_FILE_ATTRIBUTES)
{
return false;
Expand Down Expand Up @@ -1193,7 +1193,7 @@ SavingStatus FileManager::saveBuffer(BufferID id, const wchar_t* filename, bool
const wchar_t* currentBufFilePath = buffer->getFullPathName();
ULARGE_INTEGER freeBytesForUser;

BOOL getFreeSpaceSuccessful = getDiskFreeSpaceWaitSec(dirDest, &freeBytesForUser);
BOOL getFreeSpaceSuccessful = getDiskFreeSpaceWithTimeout(dirDest, &freeBytesForUser);
if (getFreeSpaceSuccessful)
{
int64_t fileSize = buffer->getFileLength();
Expand All @@ -1208,12 +1208,13 @@ SavingStatus FileManager::saveBuffer(BufferID id, const wchar_t* filename, bool
return SavingStatus::NotEnoughRoom;
}

DWORD attrib = getFileAttrWaitSec(fullpath);
if (attrib != INVALID_FILE_ATTRIBUTES)
WIN32_FILE_ATTRIBUTE_DATA attributes{};
getFileAttributesExWithTimeout(fullpath, &attributes);
if (attributes.dwFileAttributes != INVALID_FILE_ATTRIBUTES)
{
isHiddenOrSys = (attrib & (FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM)) != 0;
isHiddenOrSys = (attributes.dwFileAttributes & (FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM)) != 0;
if (isHiddenOrSys)
::SetFileAttributes(filename, attrib & ~(FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM));
::SetFileAttributes(filename, attributes.dwFileAttributes & ~(FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_SYSTEM));
}

UniMode mode = buffer->getUnicodeMode();
Expand Down Expand Up @@ -1274,7 +1275,7 @@ SavingStatus FileManager::saveBuffer(BufferID id, const wchar_t* filename, bool
}

if (isHiddenOrSys)
::SetFileAttributes(fullpath, attrib);
::SetFileAttributes(fullpath, attributes.dwFileAttributes);

if (isCopy) // "Save a Copy As..." command
{
Expand Down

0 comments on commit 3b6f22b

Please sign in to comment.