tyler.durden/notepad-plus-plus
1
0
Fork 0
mirror of https://github.com/notepad-plus-plus/notepad-plus-plus.git synced 2025-07-27 15:54:17 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix network file wrong modification detection (regression from v8.7.1)

Browse Source 
- increased DEFAULT_MILLISEC constant from 1000 to 3000 (some slower networks obviously cannot respond within the previous 1sec response timeout).
- the problematic situation was apparently made worse by the fact that the Buffer::checkFileState() did not issue one GetFileAttributesEx call but also an unnecessary second one (this was probably caused by an incorrect transcription of the previous version of this func for the new non-blocking thread method of calling the GetFileAttributesEx).
- other problem was that the checking func did not check at all whether the threaded call to the GetFileAttributesEx succeeded or had to be forcibly terminated from the main app thread and expected successful calling only.
- the nppLogNetworkDriveIssue logging has been enhanced.
- added option to check possible WIN32API error code of the threaded GetFileAttributesEx call from the main N++ thread.

Fix #15819, close #15936
This commit is contained in:
xomx 2024-12-10 20:04:14 +01:00 committed by Don Ho
parent 0c4bb240eb
commit de1a04038b
4 changed files with 138 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
PowerEditor/src/MISC/Common/Common.cpp
View File

@ -1751,7 +1751,7 @@ bool Version::isCompatibleTo(const Version& from, const Version& to) const
}
#define DEFAULT_MILLISEC 1000
#define DEFAULT_MILLISEC 3000
//----------------------------------------------------
@ -1816,6 +1816,7 @@ struct GetAttrExParamResult
wstring _filePath;
WIN32_FILE_ATTRIBUTE_DATA _attributes{};
BOOL _result = FALSE;
DWORD _error = NO_ERROR;
bool _isTimeoutReached = true;
GetAttrExParamResult(wstring filePath): _filePath(filePath) {
@ -1826,12 +1827,16 @@ struct GetAttrExParamResult
DWORD WINAPI getFileAttributesExWorker(void* data)
{
GetAttrExParamResult* inAndOut = static_cast<GetAttrExParamResult*>(data);
::SetLastError(NO_ERROR);
inAndOut->_result = ::GetFileAttributesExW(inAndOut->_filePath.c_str(), GetFileExInfoStandard, &(inAndOut->_attributes));
if (!(inAndOut->_result))
inAndOut->_error = ::GetLastError();
inAndOut->_isTimeoutReached = false;
return ERROR_SUCCESS;
};
BOOL getFileAttributesExWithTimeout(const wchar_t* filePath, WIN32_FILE_ATTRIBUTE_DATA* fileAttr, DWORD milliSec2wait, bool* isTimeoutReached)
BOOL getFileAttributesExWithTimeout(const wchar_t* filePath, WIN32_FILE_ATTRIBUTE_DATA* fileAttr,
DWORD milliSec2wait, bool* isTimeoutReached, DWORD* pdwWin32ApiError)
{
GetAttrExParamResult data(filePath);
@ -1855,13 +1860,16 @@ BOOL getFileAttributesExWithTimeout(const wchar_t* filePath, WIN32_FILE_ATTRIBUT
::TerminateThread(hThread, dwWaitStatus);
break;
}
CloseHandle(hThread);
::CloseHandle(hThread);
*fileAttr = data._attributes;
if (isTimeoutReached != nullptr)
*isTimeoutReached = data._isTimeoutReached;
if (pdwWin32ApiError != nullptr)
*pdwWin32ApiError = data._error;
return data._result;
}

6
PowerEditor/src/MISC/Common/Common.h
View File

@ -280,8 +280,10 @@ private:
};
BOOL getDiskFreeSpaceWithTimeout(const wchar_t* dirPath, ULARGE_INTEGER* freeBytesForUser, DWORD milliSec2wait = 0, bool* isTimeoutReached = nullptr);
BOOL getFileAttributesExWithTimeout(const wchar_t* filePath, WIN32_FILE_ATTRIBUTE_DATA* fileAttr, DWORD milliSec2wait = 0, bool* isTimeoutReached = nullptr);
BOOL getDiskFreeSpaceWithTimeout(const wchar_t* dirPath, ULARGE_INTEGER* freeBytesForUser,
DWORD milliSec2wait = 0, bool* isTimeoutReached = nullptr);
BOOL getFileAttributesExWithTimeout(const wchar_t* filePath, WIN32_FILE_ATTRIBUTE_DATA* fileAttr,
DWORD milliSec2wait = 0, bool* isTimeoutReached = nullptr, DWORD* pdwWin32ApiError = nullptr);
bool doesFileExist(const wchar_t* filePath, DWORD milliSec2wait = 0, bool* isTimeoutReached = nullptr);
bool doesDirectoryExist(const wchar_t* dirPath, DWORD milliSec2wait = 0, bool* isTimeoutReached = nullptr);

1
PowerEditor/src/NppBigSwitch.cpp
View File

@ -2191,7 +2191,6 @@ LRESULT Notepad_plus::process(HWND hwnd, UINT message, WPARAM wParam, LPARAM lPa
if (nppgui._fileAutoDetection != cdDisabled)
{
bool bCheckOnlyCurrentBuffer = (nppgui._fileAutoDetection & cdEnabledNew) ? true : false;
checkModifiedDocument(bCheckOnlyCurrentBuffer);
return TRUE;
}

132
PowerEditor/src/ScintillaComponent/Buffer.cpp
View File

@ -142,12 +142,49 @@ void Buffer::setLangType(LangType lang, const wchar_t* userLangName)
void Buffer::updateTimeStamp()
{
FILETIME timeStampLive {};
WIN32_FILE_ATTRIBUTE_DATA attributes{};
attributes.dwFileAttributes = INVALID_FILE_ATTRIBUTES;
if (getFileAttributesExWithTimeout(_fullPathName.c_str(), &attributes))
bool bWorkerThreadTerminated = true;
DWORD dwWin32ApiError = NO_ERROR;
BOOL bGetFileAttributesExSucceeded = getFileAttributesExWithTimeout(_fullPathName.c_str(), &attributes, 0, &bWorkerThreadTerminated, &dwWin32ApiError);
if (bGetFileAttributesExSucceeded && (attributes.dwFileAttributes != INVALID_FILE_ATTRIBUTES) && !(attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY))
{
timeStampLive = attributes.ftLastWriteTime;
}
else
{
if (_currentStatus != DOC_UNNAMED)
{
NppParameters& nppParam = NppParameters::getInstance();
if (nppParam.doNppLogNetworkDriveIssue())
{
wstring issueFn = nppLogNetworkDriveIssue;
issueFn += L".log";
wstring nppIssueLog = nppParam.getUserPath();
pathAppend(nppIssueLog, issueFn);
std::string msg = wstring2string(_fullPathName, CP_UTF8);
msg += " in Buffer::updateTimeStamp(), getFileAttributesExWithTimeout returned ";
if (bGetFileAttributesExSucceeded)
msg += "TRUE";
else
msg += "FALSE";
if (bWorkerThreadTerminated)
{
msg += ", its worker thread had to be forcefully terminated due to timeout reached!";
}
else
{
msg += ", its worker thread finished successfully within the timeout given, ";
if (attributes.dwFileAttributes == INVALID_FILE_ATTRIBUTES)
msg += "dwFileAttributes == INVALID_FILE_ATTRIBUTES ! (WIN32API Error Code: " + std::to_string(dwWin32ApiError) + ")";
else
msg += "dwFileAttributes has the FILE_ATTRIBUTE_DIRECTORY flag set!";
}
writeLog(nppIssueLog.c_str(), msg.c_str());
}
}
}
LONG res = CompareFileTime(&_timeStamp, &timeStampLive);
if (res == -1 || res == 1)
@ -156,20 +193,29 @@ void Buffer::updateTimeStamp()
// (res == 1) => timeStampLive (get directly from the file on disk) is earlier than buffer's timestamp - unusual case
// It can happen when user copies a backup of editing file somewhere-else firstly, then modifies the editing file in Notepad++ and saves it.
// Now user copies the backup back to erase the modified editing file outside Notepad++ (via Explorer).
{
if (res == 1)
{
NppParameters& nppParam = NppParameters::getInstance();
if (nppParam.doNppLogNetworkDriveIssue())
{
char buf[1024]{};
if (res == 1)
{
sprintf_s(buf, _countof(buf) - 1, " in updateTimeStamp(): timeStampLive (%lu/%lu) < _timeStamp (%lu/%lu)",
timeStampLive.dwLowDateTime, timeStampLive.dwHighDateTime, _timeStamp.dwLowDateTime, _timeStamp.dwHighDateTime);
}
else
{
// usual case, uncomment if needed to log too
//sprintf_s(buf, _countof(buf) - 1, " in updateTimeStamp(): timeStampLive (%lu/%lu) > _timeStamp (%lu/%lu)",
// timeStampLive.dwLowDateTime, timeStampLive.dwHighDateTime, _timeStamp.dwLowDateTime, _timeStamp.dwHighDateTime);
}
if (buf[0] != '\0')
{
wstring issueFn = nppLogNetworkDriveIssue;
issueFn += L".log";
wstring nppIssueLog = nppParam.getUserPath();
pathAppend(nppIssueLog, issueFn);
std::string msg = wstring2string(_fullPathName, CP_UTF8);
char buf[1024];
sprintf(buf, " in updateTimeStamp(): timeStampLive (%lu/%lu) < _timeStamp (%lu/%lu)", timeStampLive.dwLowDateTime, timeStampLive.dwHighDateTime, _timeStamp.dwLowDateTime, _timeStamp.dwHighDateTime);
msg += buf;
writeLog(nppIssueLog.c_str(), msg.c_str());
}
@ -260,10 +306,14 @@ bool Buffer::checkFileState() // returns true if the status has been changed (it
if (_currentStatus == DOC_UNNAMED || isMonitoringOn())
return false;
NppParameters& nppParam = NppParameters::getInstance();
WIN32_FILE_ATTRIBUTE_DATA attributes{};
attributes.dwFileAttributes = INVALID_FILE_ATTRIBUTES;
NppParameters& nppParam = NppParameters::getInstance();
bool fileExists = doesFileExist(_fullPathName.c_str());
bool bWorkerThreadTerminated = true;
DWORD dwWin32ApiError = NO_ERROR;
BOOL bGetFileAttributesExSucceeded = getFileAttributesExWithTimeout(_fullPathName.c_str(), &attributes, 0, &bWorkerThreadTerminated, &dwWin32ApiError);
bool fileExists = (bGetFileAttributesExSucceeded && (attributes.dwFileAttributes != INVALID_FILE_ATTRIBUTES) && !(attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY));
#ifndef _WIN64
bool isWow64Off = false;
@ -272,10 +322,42 @@ bool Buffer::checkFileState() // returns true if the status has been changed (it
nppParam.safeWow64EnableWow64FsRedirection(FALSE);
isWow64Off = true;
fileExists = doesFileExist(_fullPathName.c_str());
bGetFileAttributesExSucceeded = getFileAttributesExWithTimeout(_fullPathName.c_str(), &attributes, 0, &bWorkerThreadTerminated, &dwWin32ApiError);
fileExists = (bGetFileAttributesExSucceeded && (attributes.dwFileAttributes != INVALID_FILE_ATTRIBUTES) && !(attributes.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY));
}
#endif
if (!fileExists && nppParam.doNppLogNetworkDriveIssue())
{
wstring issueFn = nppLogNetworkDriveIssue;
issueFn += L".log";
wstring nppIssueLog = nppParam.getUserPath();
pathAppend(nppIssueLog, issueFn);
std::string msg = wstring2string(_fullPathName, CP_UTF8);
msg += " in Buffer::checkFileState(), getFileAttributesExWithTimeout returned ";
if (bGetFileAttributesExSucceeded)
{
msg += "TRUE";
}
else
{
msg += "FALSE";
}
if (bWorkerThreadTerminated)
{
msg += ", its worker thread had to be forcefully terminated due to timeout reached!";
}
else
{
msg += ", its worker thread finished successfully within the timeout given, ";
if (attributes.dwFileAttributes == INVALID_FILE_ATTRIBUTES)
msg += "dwFileAttributes == INVALID_FILE_ATTRIBUTES ! (WIN32API Error Code: " + std::to_string(dwWin32ApiError) + ")";
else
msg += "dwFileAttributes has the FILE_ATTRIBUTE_DIRECTORY flag set!";
}
writeLog(nppIssueLog.c_str(), msg.c_str());
}
bool isOK = false;
if (_currentStatus == DOC_INACCESSIBLE && !fileExists) //document is absent on its first load - we set readonly and not dirty, and make it be as document which has been deleted
{
@ -291,20 +373,17 @@ bool Buffer::checkFileState() // returns true if the status has been changed (it
{
_currentStatus = DOC_DELETED;
_isFileReadOnly = false;
_isDirty = true; //dirty sicne no match with filesystem
_isDirty = true; //dirty since no match with filesystem
_timeStamp = {};
doNotify(BufferChangeStatus | BufferChangeReadonly | BufferChangeTimestamp);
isOK = true;
}
else if (_currentStatus == DOC_DELETED && fileExists) //document has returned from its grave
{
if (GetFileAttributesEx(_fullPathName.c_str(), GetFileExInfoStandard, &attributes) != 0) // fileExists so it's safe to call GetFileAttributesEx directly
{
// fileExists==true here means that we can use the previously obtained attributes safely
_isFileReadOnly = attributes.dwFileAttributes & FILE_ATTRIBUTE_READONLY;
_currentStatus = DOC_MODIFIED;
_timeStamp = attributes.ftLastWriteTime;
if (_reloadFromDiskRequestGuard.try_lock())
{
doNotify(BufferChangeStatus | BufferChangeReadonly | BufferChangeTimestamp);
@ -312,8 +391,7 @@ bool Buffer::checkFileState() // returns true if the status has been changed (it
}
isOK = true;
}
}
else if (getFileAttributesExWithTimeout(_fullPathName.c_str(), &attributes))
else if (bGetFileAttributesExSucceeded)
{
int mask = 0; //status always 'changes', even if from modified to modified
bool isFileReadOnly = attributes.dwFileAttributes & FILE_ATTRIBUTE_READONLY;
@ -324,7 +402,6 @@ bool Buffer::checkFileState() // returns true if the status has been changed (it
}
LONG res = CompareFileTime(&_timeStamp, &attributes.ftLastWriteTime);
if (res == -1 || res == 1)
// (res == -1) => attributes.ftLastWriteTime is later, it means the file has been modified outside of Notepad++ - usual case
//
@ -332,22 +409,32 @@ bool Buffer::checkFileState() // returns true if the status has been changed (it
// It can happen when user copies a backup of editing file somewhere-else firstly, then modifies the editing file in Notepad++ and saves it.
// Now user copies the backup back to erase the modified editing file outside Notepad++ (via Explorer).
{
if (nppParam.doNppLogNetworkDriveIssue())
{
char buf[1024]{};
if (res == 1)
{
if (nppParam.doNppLogNetworkDriveIssue())
sprintf_s(buf, _countof(buf) - 1, " in checkFileState(): attributes.ftLastWriteTime (%lu/%lu) < _timeStamp (%lu/%lu)",
attributes.ftLastWriteTime.dwLowDateTime, attributes.ftLastWriteTime.dwHighDateTime, _timeStamp.dwLowDateTime, _timeStamp.dwHighDateTime);
}
else
{
// usual state after a file modification, uncomment if needed to log too
//sprintf_s(buf, _countof(buf) - 1, " in checkFileState(): attributes.ftLastWriteTime (%lu/%lu) > _timeStamp (%lu/%lu)",
// attributes.ftLastWriteTime.dwLowDateTime, attributes.ftLastWriteTime.dwHighDateTime, _timeStamp.dwLowDateTime, _timeStamp.dwHighDateTime);
}
if (buf[0] != '\0')
{
wstring issueFn = nppLogNetworkDriveIssue;
issueFn += L".log";
wstring nppIssueLog = nppParam.getUserPath();
pathAppend(nppIssueLog, issueFn);
std::string msg = wstring2string(_fullPathName, CP_UTF8);
char buf[1024];
sprintf(buf, " in checkFileState(): attributes.ftLastWriteTime (%lu/%lu) < _timeStamp (%lu/%lu)", attributes.ftLastWriteTime.dwLowDateTime, attributes.ftLastWriteTime.dwHighDateTime, _timeStamp.dwLowDateTime, _timeStamp.dwHighDateTime);
msg += buf;
writeLog(nppIssueLog.c_str(), msg.c_str());
}
}
_timeStamp = attributes.ftLastWriteTime;
mask |= BufferChangeTimestamp;
_currentStatus = DOC_MODIFIED;
@ -360,9 +447,7 @@ bool Buffer::checkFileState() // returns true if the status has been changed (it
if (_reloadFromDiskRequestGuard.try_lock())
{
doNotify(mask);
_reloadFromDiskRequestGuard.unlock();
return true;
}
}
@ -376,6 +461,7 @@ bool Buffer::checkFileState() // returns true if the status has been changed (it
nppParam.safeWow64EnableWow64FsRedirection(TRUE);
}
#endif
return isOK;
}