Fix Folder as Workspace crash and "queue overflow" issues

Fix crash of Folder as Workspace when too many directory changes happen:
Remove limit for amount of queued directory changes: use auto-reset Event instead of Semaphore.

Fix #6005, fix #5907, fix #3740, close #6005
This commit is contained in:
grisha vanika 2017-10-01 20:14:14 +03:00 committed by Don HO
parent 9d79d6c017
commit ab4cf88336
No known key found for this signature in database
GPG Key ID: 6C429F1D8D84F46E
6 changed files with 24 additions and 80 deletions

View File

@ -39,7 +39,6 @@
using namespace std; using namespace std;
DWORD WINAPI Notepad_plus::monitorFileOnChange(void * params) DWORD WINAPI Notepad_plus::monitorFileOnChange(void * params)
{ {
MonitorInfo *monitorInfo = static_cast<MonitorInfo *>(params); MonitorInfo *monitorInfo = static_cast<MonitorInfo *>(params);
@ -76,14 +75,12 @@ DWORD WINAPI Notepad_plus::monitorFileOnChange(void * params)
case WAIT_OBJECT_0 + 1: case WAIT_OBJECT_0 + 1:
// We've received a notification in the queue. // We've received a notification in the queue.
{
if (changes.CheckOverflow())
printStr(L"Queue overflowed.");
else
{ {
DWORD dwAction; DWORD dwAction;
CStringW wstrFilename; CStringW wstrFilename;
changes.Pop(dwAction, wstrFilename); // Process all available changes, ignore User actions
while (changes.Pop(dwAction, wstrFilename))
{
generic_string fn = wstrFilename.GetString(); generic_string fn = wstrFilename.GetString();
// Fix monitoring files which are under root problem // Fix monitoring files which are under root problem

View File

@ -1354,11 +1354,9 @@ DWORD WINAPI FolderUpdater::watching(void *params)
{ {
DWORD dwAction; DWORD dwAction;
CStringW wstrFilename; CStringW wstrFilename;
if (changes.CheckOverflow()) // Process all available changes, ignore User actions
printStr(L"Queue overflowed."); while (changes.Pop(dwAction, wstrFilename))
else
{ {
changes.Pop(dwAction, wstrFilename);
static generic_string oldName; static generic_string oldName;
static std::vector<generic_string> file2Change; static std::vector<generic_string> file2Change;
file2Change.clear(); file2Change.clear();

View File

@ -34,8 +34,8 @@ using namespace ReadDirectoryChangesPrivate;
/////////////////////////////////////////////////////////////////////////// ///////////////////////////////////////////////////////////////////////////
// CReadDirectoryChanges // CReadDirectoryChanges
CReadDirectoryChanges::CReadDirectoryChanges(int nMaxCount) CReadDirectoryChanges::CReadDirectoryChanges()
: m_Notifications(nMaxCount) : m_Notifications()
{ {
m_hThread = NULL; m_hThread = NULL;
m_dwThreadId= 0; m_dwThreadId= 0;
@ -102,11 +102,3 @@ bool CReadDirectoryChanges::Pop(DWORD& dwAction, CStringW& wstrFilename)
return true; return true;
} }
bool CReadDirectoryChanges::CheckOverflow()
{
bool b = m_Notifications.overflow();
if (b)
m_Notifications.clear();
return b;
}

View File

@ -99,7 +99,7 @@ namespace ReadDirectoryChangesPrivate
/// { /// {
/// DWORD dwAction; /// DWORD dwAction;
/// CStringW wstrFilename; /// CStringW wstrFilename;
/// changes.Pop(dwAction, wstrFilename); /// while (changes.Pop(dwAction, wstrFilename))
/// wprintf(L"%s %s\n", ExplainAction(dwAction), wstrFilename); /// wprintf(L"%s %s\n", ExplainAction(dwAction), wstrFilename);
/// } /// }
/// break; /// break;
@ -116,7 +116,7 @@ namespace ReadDirectoryChangesPrivate
class CReadDirectoryChanges class CReadDirectoryChanges
{ {
public: public:
CReadDirectoryChanges(int nMaxChanges=1000); CReadDirectoryChanges();
~CReadDirectoryChanges(); ~CReadDirectoryChanges();
void Init(); void Init();
@ -148,9 +148,6 @@ public:
// "Push" is for usage by ReadChangesRequest. Not intended for external usage. // "Push" is for usage by ReadChangesRequest. Not intended for external usage.
void Push(DWORD dwAction, CStringW& wstrFilename); void Push(DWORD dwAction, CStringW& wstrFilename);
// Check if the queue overflowed. If so, clear it and return true.
bool CheckOverflow();
unsigned int GetThreadId() { return m_dwThreadId; } unsigned int GetThreadId() { return m_dwThreadId; }
protected: protected:

View File

@ -123,11 +123,8 @@ 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.
_ASSERTE(dwNumberOfBytesTransfered >= offsetof(FILE_NOTIFY_INFORMATION, FileName) + sizeof(WCHAR)); _ASSERTE((dwNumberOfBytesTransfered == 0) ||
(dwNumberOfBytesTransfered >= offsetof(FILE_NOTIFY_INFORMATION, FileName) + sizeof(WCHAR)));
// This might mean overflow? Not sure.
if (!dwNumberOfBytesTransfered)
return;
pBlock->BackupBuffer(dwNumberOfBytesTransfered); pBlock->BackupBuffer(dwNumberOfBytesTransfered);

View File

@ -32,52 +32,35 @@ template <typename C>
class CThreadSafeQueue : protected std::list<C> class CThreadSafeQueue : protected std::list<C>
{ {
public: public:
CThreadSafeQueue(int nMaxCount) CThreadSafeQueue()
{ {
m_bOverflow = false; m_hEvent = ::CreateEvent(
m_hSemaphore = ::CreateSemaphore(
NULL, // no security attributes NULL, // no security attributes
0, // initial count FALSE, // auto reset
nMaxCount, // max count FALSE, // non-signalled
NULL); // anonymous NULL); // anonymous
} }
~CThreadSafeQueue() ~CThreadSafeQueue()
{ {
::CloseHandle(m_hSemaphore); ::CloseHandle(m_hEvent);
m_hSemaphore = NULL; m_hEvent = NULL;
} }
void push(C& c) void push(C& c)
{
{ {
CComCritSecLock<CComAutoCriticalSection> lock(m_Crit, true); CComCritSecLock<CComAutoCriticalSection> lock(m_Crit, true);
push_back(c); push_back(c);
lock.Unlock();
if (!::ReleaseSemaphore(m_hSemaphore, 1, NULL))
{
// If the semaphore is full, then take back the entry.
lock.Lock();
pop_back();
if (GetLastError() == ERROR_TOO_MANY_POSTS)
{
m_bOverflow = true;
}
} }
::SetEvent(m_hEvent);
} }
bool pop(C& c) bool pop(C& c)
{ {
CComCritSecLock<CComAutoCriticalSection> lock( m_Crit, true ); CComCritSecLock<CComAutoCriticalSection> lock( m_Crit, true );
// If the user calls pop() more than once after the
// semaphore is signaled, then the semaphore count will
// get out of sync. We fix that when the queue empties.
if (empty()) if (empty())
{ {
while (::WaitForSingleObject(m_hSemaphore, 0) != WAIT_TIMEOUT)
1;
return false; return false;
} }
@ -87,30 +70,10 @@ public:
return true; return true;
} }
// If overflow, use this to clear the queue. HANDLE GetWaitHandle() { return m_hEvent; }
void clear()
{
CComCritSecLock<CComAutoCriticalSection> lock( m_Crit, true );
for (DWORD i=0; i<size(); i++)
WaitForSingleObject(m_hSemaphore, 0);
__super::clear();
m_bOverflow = false;
}
bool overflow()
{
return m_bOverflow;
}
HANDLE GetWaitHandle() { return m_hSemaphore; }
protected: protected:
HANDLE m_hSemaphore; HANDLE m_hEvent;
CComAutoCriticalSection m_Crit; CComAutoCriticalSection m_Crit;
bool m_bOverflow;
}; };