diff --git a/include/wx/msw/private/fswatcher.h b/include/wx/msw/private/fswatcher.h index 5d2e6f86a3..17458b47cf 100644 --- a/include/wx/msw/private/fswatcher.h +++ b/include/wx/msw/private/fswatcher.h @@ -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; diff --git a/src/msw/fswatcher.cpp b/src/msw/fswatcher.cpp index 3db4632204..29ed145745 100644 --- a/src/msw/fswatcher.cpp +++ b/src/msw/fswatcher.cpp @@ -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 events; const char* memory = static_cast(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);