Browse Source

Reduce network file hanging issue due to win32API GetFileAttributes cache (unsynchronized).

STR:
1. Open a network file.
2. Close Notepad++ to have it in the session.
3. Disconnect the network, and launch Notepad++ immediately.
4. Around more than 1 minute's delay, then the "Error" dialog displayed.

The reason for the hanging is that the network file was incorrectly detected by doesFileExist (GetFileAttributesEx) as present, leading Notepad++ to attempt opening a non-existent file with _wfopen. This issue seems to stem from a caching mechanism within the IO function (GetFileAttributesEx). When the network disconnects, the cache is not immediately cleared, causing GetFileAttributesEx to falsely report that the file exists. Consequently, when Notepad++ is launched after a network disconnection, GetFileAttributesEx retains its cache, indicating the file exists, while _wfopen fails to locate the network resource, resulting in a hang.

Unfortunately, there's no efficient solution for this problem. The commit's remedy is to check if the file is on the network and whether its directory still exists. If the directory doesn't exist, we avoid calling _wfopen. We verify the directory's existence instead of the file itself because the cache issue with GetFileAttributesEx occurs before _wfopen is executed. Checking the directory avoids the cache problem due to the identical argument being used.

I've tested this remedy in debug mode, and it works fine. However, the problem persists in release mode. Despite this, I believe it's worth keeping this solution, as it provides some protection in a variable network environment, potentially mitigating the issue when it arises.

Ref: https://github.com/notepad-plus-plus/notepad-plus-plus/pull/15658#issuecomment-2386662974

Improve #4306, #6178, #8055, #11388, #12553, #15540, close #15701
pull/15711/head
Don Ho 1 month ago
parent
commit
dc9f58ddac
  1. 2
      PowerEditor/src/MISC/Common/Common.cpp
  2. 2
      PowerEditor/src/MISC/Common/Common.h
  3. 4
      PowerEditor/src/MISC/Common/FileInterface.cpp
  4. 4
      PowerEditor/src/Notepad_plus.cpp
  5. 4
      PowerEditor/src/NppBigSwitch.cpp
  6. 2
      PowerEditor/src/NppCommands.cpp
  7. 10
      PowerEditor/src/NppIO.cpp
  8. 21
      PowerEditor/src/ScintillaComponent/Buffer.cpp
  9. 2
      PowerEditor/src/ScintillaComponent/FindReplaceDlg.cpp
  10. 26
      PowerEditor/src/ScintillaComponent/ScintillaEditView.cpp
  11. 2
      PowerEditor/src/WinControls/ReadDirectoryChanges/ReadDirectoryChangesPrivate.cpp

2
PowerEditor/src/MISC/Common/Common.cpp

@ -629,7 +629,7 @@ wstring BuildMenuFileName(int filenameLen, unsigned int pos, const wstring &file
} }
wstring PathRemoveFileSpec(wstring& path) wstring pathRemoveFileSpec(wstring& path)
{ {
wstring::size_type lastBackslash = path.find_last_of(L'\\'); wstring::size_type lastBackslash = path.find_last_of(L'\\');
if (lastBackslash == wstring::npos) if (lastBackslash == wstring::npos)

2
PowerEditor/src/MISC/Common/Common.h

@ -154,7 +154,7 @@ protected:
#define REBARBAND_SIZE sizeof(REBARBANDINFO) #define REBARBAND_SIZE sizeof(REBARBANDINFO)
std::wstring PathRemoveFileSpec(std::wstring & path); std::wstring pathRemoveFileSpec(std::wstring & path);
std::wstring pathAppend(std::wstring &strDest, const std::wstring & str2append); std::wstring pathAppend(std::wstring &strDest, const std::wstring & str2append);
COLORREF getCtrlBgColor(HWND hWnd); COLORREF getCtrlBgColor(HWND hWnd);
std::wstring stringToUpper(std::wstring strToConvert); std::wstring stringToUpper(std::wstring strToConvert);

4
PowerEditor/src/MISC/Common/FileInterface.cpp

@ -37,7 +37,7 @@ Win32_IO_File::Win32_IO_File(const wchar_t *fname)
// Store the file creation date & attributes for a possible use later... // Store the file creation date & attributes for a possible use later...
if (getFileAttributesExWithTimeout(fname, &attributes_original, 0, &isTimeoutReached)) if (getFileAttributesExWithTimeout(fname, &attributes_original, 0, &isTimeoutReached))
{ {
fileExists = (attributes_original.dwFileAttributes != INVALID_FILE_ATTRIBUTES); fileExists = (attributes_original.dwFileAttributes != INVALID_FILE_ATTRIBUTES && !(attributes_original.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY));
} }
if (fileExists) if (fileExists)
@ -120,7 +120,7 @@ void Win32_IO_File::close()
std::wstring curFilePath; std::wstring curFilePath;
const DWORD cchPathBuf = MAX_PATH + 128; const DWORD cchPathBuf = MAX_PATH + 128;
WCHAR pathbuf[cchPathBuf]{}; wchar_t pathbuf[cchPathBuf]{};
// the dwFlags used below are the most error-proof and informative // the dwFlags used below are the most error-proof and informative
DWORD dwRet = ::GetFinalPathNameByHandle(_hFile, pathbuf, cchPathBuf, FILE_NAME_OPENED | VOLUME_NAME_NT); DWORD dwRet = ::GetFinalPathNameByHandle(_hFile, pathbuf, cchPathBuf, FILE_NAME_OPENED | VOLUME_NAME_NT);
if ((dwRet == 0) || (dwRet >= cchPathBuf)) if ((dwRet == 0) || (dwRet >= cchPathBuf))

4
PowerEditor/src/Notepad_plus.cpp

@ -6679,7 +6679,7 @@ void Notepad_plus::notifyBufferChanged(Buffer * buffer, int mask)
checkDocState(); checkDocState();
setTitle(); setTitle();
wstring dir(buffer->getFullPathName()); wstring dir(buffer->getFullPathName());
PathRemoveFileSpec(dir); pathRemoveFileSpec(dir);
setWorkingDir(dir.c_str()); setWorkingDir(dir.c_str());
} }
@ -6744,7 +6744,7 @@ void Notepad_plus::notifyBufferActivated(BufferID bufid, int view)
setDisplayFormat(buf->getEolFormat()); setDisplayFormat(buf->getEolFormat());
enableConvertMenuItems(buf->getEolFormat()); enableConvertMenuItems(buf->getEolFormat());
wstring dir(buf->getFullPathName()); wstring dir(buf->getFullPathName());
PathRemoveFileSpec(dir); pathRemoveFileSpec(dir);
setWorkingDir(dir.c_str()); setWorkingDir(dir.c_str());
setTitle(); setTitle();
//Make sure the colors of the tab controls match //Make sure the colors of the tab controls match

4
PowerEditor/src/NppBigSwitch.cpp

@ -78,7 +78,7 @@ bool SetOSAppRestart()
pathAppend(nppIssueLog, issueFn); pathAppend(nppIssueLog, issueFn);
} }
WCHAR wszCmdLine[RESTART_MAX_CMD_LINE] = { 0 }; wchar_t wszCmdLine[RESTART_MAX_CMD_LINE] = { 0 };
DWORD cchCmdLine = _countof(wszCmdLine); DWORD cchCmdLine = _countof(wszCmdLine);
DWORD dwPreviousFlags = 0; DWORD dwPreviousFlags = 0;
HRESULT hr = ::GetApplicationRestartSettings(::GetCurrentProcess(), wszCmdLine, &cchCmdLine, &dwPreviousFlags); HRESULT hr = ::GetApplicationRestartSettings(::GetCurrentProcess(), wszCmdLine, &cchCmdLine, &dwPreviousFlags);
@ -3744,7 +3744,7 @@ LRESULT Notepad_plus::process(HWND hwnd, UINT message, WPARAM wParam, LPARAM lPa
{ {
const Buffer* buf = _pEditView->getCurrentBuffer(); const Buffer* buf = _pEditView->getCurrentBuffer();
wstring path = buf ? buf->getFullPathName() : L""; wstring path = buf ? buf->getFullPathName() : L"";
PathRemoveFileSpec(path); pathRemoveFileSpec(path);
setWorkingDir(path.c_str()); setWorkingDir(path.c_str());
return TRUE; return TRUE;
} }

2
PowerEditor/src/NppCommands.cpp

@ -1281,7 +1281,7 @@ void Notepad_plus::command(int id)
else if (id == IDM_EDIT_CURRENTDIRTOCLIP) else if (id == IDM_EDIT_CURRENTDIRTOCLIP)
{ {
wstring dir(buf->getFullPathName()); wstring dir(buf->getFullPathName());
PathRemoveFileSpec(dir); pathRemoveFileSpec(dir);
str2Cliboard(dir); str2Cliboard(dir);
} }
else if (id == IDM_EDIT_FILENAMETOCLIP) else if (id == IDM_EDIT_FILENAMETOCLIP)

10
PowerEditor/src/NppIO.cpp

@ -46,7 +46,7 @@ DWORD WINAPI Notepad_plus::monitorFileOnChange(void * params)
const wchar_t *fullFileName = (const wchar_t *)buf->getFullPathName(); const wchar_t *fullFileName = (const wchar_t *)buf->getFullPathName();
//The folder to watch : //The folder to watch :
WCHAR folderToMonitor[MAX_PATH]{}; wchar_t folderToMonitor[MAX_PATH]{};
wcscpy_s(folderToMonitor, fullFileName); wcscpy_s(folderToMonitor, fullFileName);
::PathRemoveFileSpecW(folderToMonitor); ::PathRemoveFileSpecW(folderToMonitor);
@ -143,7 +143,7 @@ bool resolveLinkFile(std::wstring& linkFilePath)
bool isResolved = false; bool isResolved = false;
IShellLink* psl = nullptr; IShellLink* psl = nullptr;
WCHAR targetFilePath[MAX_PATH]{}; wchar_t targetFilePath[MAX_PATH]{};
WIN32_FIND_DATA wfd{}; WIN32_FIND_DATA wfd{};
HRESULT hres = CoInitialize(NULL); HRESULT hres = CoInitialize(NULL);
@ -234,7 +234,7 @@ BufferID Notepad_plus::doOpen(const wstring& fileName, bool isRecursive, bool is
//If [GetFullPathName] fails for any other reason, the return value is zero. //If [GetFullPathName] fails for any other reason, the return value is zero.
NppParameters& nppParam = NppParameters::getInstance(); NppParameters& nppParam = NppParameters::getInstance();
WCHAR longFileName[longFileNameBufferSize] = { 0 }; wchar_t longFileName[longFileNameBufferSize] = { 0 };
if (isRawFileName) if (isRawFileName)
{ {
@ -337,7 +337,7 @@ BufferID Notepad_plus::doOpen(const wstring& fileName, bool isRecursive, bool is
if (!doesPathExist(longFileName) && !globbing) if (!doesPathExist(longFileName) && !globbing)
{ {
wstring longFileDir(longFileName); wstring longFileDir(longFileName);
PathRemoveFileSpec(longFileDir); pathRemoveFileSpec(longFileDir);
bool isCreateFileSuccessful = false; bool isCreateFileSuccessful = false;
if (doesDirectoryExist(longFileDir.c_str())) if (doesDirectoryExist(longFileDir.c_str()))
@ -1674,7 +1674,7 @@ bool Notepad_plus::fileSave(BufferID id)
{ {
// Get the current file's directory // Get the current file's directory
wstring path = fn; wstring path = fn;
::PathRemoveFileSpec(path); ::pathRemoveFileSpec(path);
fn_bak = path; fn_bak = path;
fn_bak += L"\\"; fn_bak += L"\\";

21
PowerEditor/src/ScintillaComponent/Buffer.cpp

@ -747,7 +747,7 @@ BufferID FileManager::loadFile(const wchar_t* filename, Document doc, int encodi
ownDoc = true; ownDoc = true;
} }
WCHAR fullpath[MAX_PATH] = { 0 }; wchar_t fullpath[MAX_PATH] = { 0 };
if (isWin32NamespacePrefixedFileName(filename)) // This function checks for the \\?\ prefix if (isWin32NamespacePrefixedFileName(filename)) // This function checks for the \\?\ prefix
{ {
// use directly the raw file name, skip the GetFullPathName WINAPI // use directly the raw file name, skip the GetFullPathName WINAPI
@ -1171,7 +1171,7 @@ SavingStatus FileManager::saveBuffer(BufferID id, const wchar_t* filename, bool
Buffer* buffer = getBufferByID(id); Buffer* buffer = getBufferByID(id);
bool isHiddenOrSys = false; bool isHiddenOrSys = false;
WCHAR fullpath[MAX_PATH] = { 0 }; wchar_t fullpath[MAX_PATH] = { 0 };
if (isWin32NamespacePrefixedFileName(filename)) if (isWin32NamespacePrefixedFileName(filename))
{ {
// use directly the raw file name, skip the GetFullPathName WINAPI // use directly the raw file name, skip the GetFullPathName WINAPI
@ -1615,6 +1615,23 @@ bool FileManager::loadFileData(Document doc, int64_t fileSize, const wchar_t * f
} }
} }
// Check if the file is located on a network. If it is, verify the existence of the file's directory.
// Note: We're checking the directory's existence instead of the file itself to avoid the GetAttributesEx cache issue.
// Just before calling loadFileData, the doesFileExist function was called, which should return false if there's a network problem.
// If execution reaches here during a network connection problem, it means doesFileExist returned true incorrectly due to GetAttributesEx caching.
// Therefore, we avoid calling doesFileExist again and instead call doesDirectoryExist to ensure accuracy.
bool isNetworkDirDisconnected = false;
if (PathIsNetworkPath(filename))
{
wchar_t dir[MAX_PATH]{};
wcscpy_s(dir,filename);
PathRemoveFileSpec(dir);
isNetworkDirDisconnected = !doesDirectoryExist(dir);
}
if (isNetworkDirDisconnected)
return false; // If network ressource is not reachable, we stop here for not having hanging issue because of _wfopen
FILE* fp = _wfopen(filename, L"rb"); FILE* fp = _wfopen(filename, L"rb");
if (!fp) if (!fp)

2
PowerEditor/src/ScintillaComponent/FindReplaceDlg.cpp

@ -2646,7 +2646,7 @@ intptr_t CALLBACK FindReplaceDlg::run_dlgProc(UINT message, WPARAM wParam, LPARA
if (!(buf->getStatus() & (DOC_UNNAMED | DOC_DELETED))) if (!(buf->getStatus() & (DOC_UNNAMED | DOC_DELETED)))
{ {
currPath = buf->getFullPathName(); currPath = buf->getFullPathName();
PathRemoveFileSpec(currPath); pathRemoveFileSpec(currPath);
} }
if (currPath.empty() || !doesDirectoryExist(currPath.c_str())) if (currPath.empty() || !doesDirectoryExist(currPath.c_str()))
currPath = NppParameters::getInstance().getWorkingDir(); currPath = NppParameters::getInstance().getWorkingDir();

26
PowerEditor/src/ScintillaComponent/ScintillaEditView.cpp

@ -462,7 +462,7 @@ LRESULT ScintillaEditView::scintillaNew_Proc(HWND hwnd, UINT Message, WPARAM wPa
{ {
// convert the selection to Unicode, and get the number // convert the selection to Unicode, and get the number
// of bytes required for the converted text // of bytes required for the converted text
textLength = sizeof(WCHAR) * ::MultiByteToWideChar(codepage, 0, selectedStr, (int)selectSize, NULL, 0); textLength = sizeof(wchar_t) * ::MultiByteToWideChar(codepage, 0, selectedStr, (int)selectSize, NULL, 0);
} }
else else
{ {
@ -485,7 +485,7 @@ LRESULT ScintillaEditView::scintillaNew_Proc(HWND hwnd, UINT Message, WPARAM wPa
reconvert->dwTargetStrLen = reconvert->dwCompStrLen; reconvert->dwTargetStrLen = reconvert->dwCompStrLen;
reconvert->dwTargetStrOffset = reconvert->dwCompStrOffset; reconvert->dwTargetStrOffset = reconvert->dwCompStrOffset;
textLength *= sizeof(WCHAR); textLength *= sizeof(wchar_t);
} }
if (selectedStr != smallTextBuffer) if (selectedStr != smallTextBuffer)
@ -3535,7 +3535,7 @@ void ScintillaEditView::changeCase(__inout wchar_t * const strWToConvert, const
{ {
for (int i = 0; i < nbChars; ++i) for (int i = 0; i < nbChars; ++i)
{ {
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharUpperW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharUpperW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
} }
break; break;
} //case UPPERCASE } //case UPPERCASE
@ -3543,7 +3543,7 @@ void ScintillaEditView::changeCase(__inout wchar_t * const strWToConvert, const
{ {
for (int i = 0; i < nbChars; ++i) for (int i = 0; i < nbChars; ++i)
{ {
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
} }
break; break;
} //case LOWERCASE } //case LOWERCASE
@ -3559,12 +3559,12 @@ void ScintillaEditView::changeCase(__inout wchar_t * const strWToConvert, const
(isCharSingleQuote(strWToConvert[i - 1]) && ::IsCharAlphaNumericW(strWToConvert[i - 2]))) (isCharSingleQuote(strWToConvert[i - 1]) && ::IsCharAlphaNumericW(strWToConvert[i - 2])))
{ {
if (caseToConvert == PROPERCASE_FORCE) if (caseToConvert == PROPERCASE_FORCE)
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
} }
else if ((i < 1) ? true : !::IsCharAlphaNumericW(strWToConvert[i - 1])) else if ((i < 1) ? true : !::IsCharAlphaNumericW(strWToConvert[i - 1]))
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharUpperW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharUpperW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
else if (caseToConvert == PROPERCASE_FORCE) else if (caseToConvert == PROPERCASE_FORCE)
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
} }
} }
break; break;
@ -3581,12 +3581,12 @@ void ScintillaEditView::changeCase(__inout wchar_t * const strWToConvert, const
{ {
if (isNewSentence) if (isNewSentence)
{ {
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharUpperW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharUpperW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
isNewSentence = false; isNewSentence = false;
} }
else if (caseToConvert == SENTENCECASE_FORCE) else if (caseToConvert == SENTENCECASE_FORCE)
{ {
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
} }
wasEolR = false; wasEolR = false;
wasEolN = false; wasEolN = false;
@ -3627,9 +3627,9 @@ void ScintillaEditView::changeCase(__inout wchar_t * const strWToConvert, const
for (int i = 0; i < nbChars; ++i) for (int i = 0; i < nbChars; ++i)
{ {
if (::IsCharLowerW(strWToConvert[i])) if (::IsCharLowerW(strWToConvert[i]))
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharUpperW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharUpperW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
else else
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
} }
break; break;
} //case INVERTCASE } //case INVERTCASE
@ -3640,9 +3640,9 @@ void ScintillaEditView::changeCase(__inout wchar_t * const strWToConvert, const
if (::IsCharAlphaW(strWToConvert[i])) if (::IsCharAlphaW(strWToConvert[i]))
{ {
if (std::rand() & true) if (std::rand() & true)
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharUpperW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharUpperW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
else else
strWToConvert[i] = (WCHAR)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i])); strWToConvert[i] = (wchar_t)(UINT_PTR)::CharLowerW(reinterpret_cast<LPWSTR>(strWToConvert[i]));
} }
} }
break; break;

2
PowerEditor/src/WinControls/ReadDirectoryChanges/ReadDirectoryChangesPrivate.cpp

@ -121,7 +121,7 @@ VOID CALLBACK CReadChangesRequest::NotificationCompletion(
// Can't use sizeof(FILE_NOTIFY_INFORMATION) because // Can't use sizeof(FILE_NOTIFY_INFORMATION) because
// the structure is padded to 16 bytes. // the structure is padded to 16 bytes.
assert((dwNumberOfBytesTransfered == 0) || assert((dwNumberOfBytesTransfered == 0) ||
(dwNumberOfBytesTransfered >= offsetof(FILE_NOTIFY_INFORMATION, FileName) + sizeof(WCHAR))); (dwNumberOfBytesTransfered >= offsetof(FILE_NOTIFY_INFORMATION, FileName) + sizeof(wchar_t)));
pBlock->BackupBuffer(dwNumberOfBytesTransfered); pBlock->BackupBuffer(dwNumberOfBytesTransfered);

Loading…
Cancel
Save