Mirrors_UI_SDK/wxWidgets
1
0
Fork 0
mirror of https://github.com/wxWidgets/wxWidgets.git synced 2026-10-06 07:00:16 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix MSW wxFileSystemWatcher removal races

Browse Source 
Cancel outstanding overlapped directory reads before destroying their
buffers, and protect the IOCP watch lists shared between the main thread
and worker thread. Also handle cancelled completions as normal watch
removal so removed watches are not processed or reissued.

Fixes #22381.

Closes #26866.
This commit is contained in:
Richard
2026-09-03 23:35:58 +02:00
committed by Vadim Zeitlin
parent 80ed17db3b
commit 53e793af35
2 changed files with 94 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
include/wx/msw/private/fswatcher.h
+82 -9
View File
@@ -11,6 +11,7 @@
#define WX_MSW_PRIVATE_FSWATCHER_H_
#include "wx/filename.h"
#include "wx/thread.h"
#include "wx/vector.h"
#include "wx/msw/private.h"
@@ -34,6 +35,15 @@ public:
// get handle for this path
m_handle = OpenDir(m_path);
m_overlapped = (OVERLAPPED*)calloc(1, sizeof(OVERLAPPED));
// The IOCP receives read completions, but the event gives
// GetOverlappedResult(..., TRUE) a handle to wait on after CancelIoEx(),
// so the OVERLAPPED object and buffer are not freed until the pending
// ReadDirectoryChangesW() call has really finished.
m_overlapped->hEvent = ::CreateEvent(nullptr, TRUE, FALSE, nullptr);
if ( !m_overlapped->hEvent )
{
wxLogLastError("Failed to create event for path monitoring");
}
wxZeroMemory(m_buffer);
}
@@ -43,18 +53,65 @@ public:
if (m_handle != INVALID_HANDLE_VALUE)
{
if (!CloseHandle(m_handle))
if ( m_overlapped )
{
wxLogSysError(_("Unable to close the handle for '%s'"),
m_path);
// The event buffer and OVERLAPPED object must remain alive
// until the asynchronous read has really completed.
if ( !::CancelIoEx(m_handle, m_overlapped) )
{
const DWORD err = ::GetLastError();
// ERROR_NOT_FOUND means the asynchronous read isn't
// pending any more. This can happen on the normal removal
// path, after the IOCP worker dequeues its completion and
// drops the final reference to this watch. There is then
// nothing left for GetOverlappedResult(..., TRUE) to wait
// for before freeing the OVERLAPPED object and buffer.
if ( err != ERROR_NOT_FOUND )
{
wxLogDebug("Unable to cancel the watch for '%s': %s",
m_path, wxSysErrorMsgStr(err));
}
}
else
{
DWORD bytes = 0;
if ( !::GetOverlappedResult(m_handle, m_overlapped, &bytes,
TRUE /* wait */) )
{
const DWORD err = ::GetLastError();
if ( err != ERROR_OPERATION_ABORTED )
{
wxLogSysError(err,
_("Unable to finish cancelling the "
"watch for '%s'"),
m_path);
}
}
}
}
if ( !::CloseHandle(m_handle) )
{
wxLogDebug("Unable to close the watch handle for '%s': %s",
m_path, wxSysErrorMsgStr());
}
}
free(m_overlapped);
if ( m_overlapped )
{
if ( m_overlapped->hEvent && !::CloseHandle(m_overlapped->hEvent) )
{
wxLogDebug("Unable to close the watch event for '%s': %s",
m_path, wxSysErrorMsgStr());
}
free(m_overlapped);
}
}
bool IsOk() const
{
return m_handle != INVALID_HANDLE_VALUE;
return m_handle != INVALID_HANDLE_VALUE &&
m_overlapped && m_overlapped->hEvent;
}
HANDLE GetHandle() const
@@ -118,6 +175,9 @@ public:
~wxIOCPService()
{
m_watches.clear();
m_removedWatches.clear();
if (m_iocp != INVALID_HANDLE_VALUE)
{
if (!CloseHandle(m_iocp))
@@ -125,7 +185,6 @@ public:
wxLogSysError(_("Unable to close I/O completion port handle"));
}
}
m_watches.clear();
}
// associates a wxFSWatchEntryMSW with completion port
@@ -134,6 +193,8 @@ public:
wxCHECK_MSG( m_iocp != INVALID_HANDLE_VALUE, false, "IOCP not init" );
wxCHECK_MSG( watch->IsOk(), false, "Invalid watch" );
wxCriticalSectionLocker lock(m_critsect);
// associate with IOCP
HANDLE ret = CreateIoCompletionPort(watch->GetHandle(), m_iocp,
(ULONG_PTR)watch.get(), 0);
@@ -163,6 +224,8 @@ public:
wxCHECK_MSG( m_iocp != INVALID_HANDLE_VALUE, false, "IOCP not init" );
wxCHECK_MSG( watch->IsOk(), false, "Invalid watch" );
wxCriticalSectionLocker lock(m_critsect);
const wxString path = watch->GetPath();
wxFSWatchEntries::iterator it = m_watches.find(path);
wxCHECK_MSG( it != m_watches.end(), false,
@@ -185,6 +248,8 @@ public:
// this case we'll just return false and do nothing.
bool CompleteRemoval(wxFSWatchEntryMSW* watch)
{
wxCriticalSectionLocker lock(m_critsect);
for ( Watches::iterator it = m_removedWatches.begin();
it != m_removedWatches.end();
++it )
@@ -251,15 +316,20 @@ public:
return *count || *watch || *overlapped ? Status_OK : Status_Exit;
}
const DWORD err = wxSysErrorCode();
if ( err == ERROR_OPERATION_ABORTED && *watch )
return Status_OK;
// An error is returned if the underlying directory has been deleted,
// but this is not really an unexpected failure, so handle it
// specially.
if ( wxSysErrorCode() == ERROR_ACCESS_DENIED &&
*watch && !wxFileName::DirExists((*watch)->GetPath()) )
if ( err == ERROR_ACCESS_DENIED &&
*watch && !wxFileName::DirExists((*watch)->GetPath()) )
return Status_Deleted;
// Some other error, at least log it.
wxLogSysError(_("Unable to dequeue completion packet"));
wxLogSysError(err, _("Unable to dequeue completion packet"));
return Status_Error;
}
@@ -277,6 +347,9 @@ protected:
HANDLE m_iocp;
// Protects the watch lists shared by the user thread and IOCP thread.
wxCriticalSection m_critsect;
// The hash containing all the wxFSWatchEntryMSW objects currently being
// watched keyed by their paths.
wxFSWatchEntries m_watches;
src/msw/fswatcher.cpp
+12 -5
View File
@@ -155,6 +155,7 @@ bool wxFSWatcherImplMSW::DoSetUpWatch(wxFSWatchEntryMSW& watch)
}
int flags = Watcher2NativeFlags(watch.GetFlags());
::ResetEvent(watch.GetOverlapped()->hEvent);
int ret = ReadDirectoryChangesW(watch.GetHandle(), watch.GetBuffer(),
wxFSWatchEntryMSW::BUFFER_SIZE,
bWatchSubtree,
@@ -230,6 +231,9 @@ bool wxIOCPThread::ReadEvents()
return true; // error was logged already, we don't want to exit
case wxIOCPService::Status_Deleted:
if ( m_iocp->CompleteRemoval(watch) )
return true;
{
wxFileSystemWatcherEvent
removeEvent(wxFSW_EVENT_DELETE,
@@ -248,6 +252,11 @@ bool wxIOCPThread::ReadEvents()
return false; // stop reading events
}
// First check if we're still interested in this watch, we could have
// removed it in the meanwhile.
if ( watch && m_iocp->CompleteRemoval(watch) )
return true;
// if the thread got woken up but we got an empty packet it means that
// there was an overflow, too many events and not all could fit in
// the watch buffer. In this case, ReadDirectoryChangesW dumps the
@@ -278,11 +287,6 @@ bool wxIOCPThread::ReadEvents()
wxLogTrace( wxTRACE_FSWATCHER, "[iocp] Read entry: path='%s'",
watch->GetPath());
// First check if we're still interested in this watch, we could have
// removed it in the meanwhile.
if ( m_iocp->CompleteRemoval(watch) )
return true;
// extract events from buffer info our vector container
wxVector<wxEventProcessingData> events;
const char* memory = static_cast<const char*>(watch->GetBuffer());
@@ -302,6 +306,9 @@ bool wxIOCPThread::ReadEvents()
// process events
ProcessNativeEvents(events);
if ( m_iocp->CompleteRemoval(watch) )
return true;
// reissue the watch. ignore possible errors, we will return true anyway
(void) m_service->SetUpWatch(*watch);