mirror of
https://github.com/wxWidgets/wxWidgets.git
synced 2026-10-06 07:00:16 +08:00
got3nks
ddd1e72ee4
Dispatch() stored the wxFDIOHandler pointer in epoll_event::data.ptr and called through the copy that epoll_wait() had filled in before any handler ran. Servicing one event of a batch can unregister the descriptor belonging to a later event of the same batch -- and, in the code owning it, destroy its handler with it -- but nothing scrubs the pointer already copied into the array, so the loop went on to make a virtual call on released memory. Store the descriptor in epoll_event::data instead and look the handler up when the event is actually processed, so that one which is no longer registered is skipped rather than called. That is what wxSelectDispatcher has always done, via FindHandler() in ProcessSets(). The map this needs is kept here rather than by deriving from wxMappedFDIODispatcher, for two reasons: - Its ModifyFD() asserts that the descriptor is already known, but wxFDIOManagerUnix chooses between RegisterFD() and ModifyFD() from the mask it keeps on the handler rather than from anything this dispatcher knows, so the two can legitimately disagree. epoll_ctl(EPOLL_CTL_MOD) merely reports ENOENT in that case, and that tolerance has to be preserved. Recording the handler is therefore an unconditional assignment. - Descriptors are registered and unregistered from worker threads, e.g. by wxSocketImpl from whichever thread performs the socket operation, while Dispatch() reads the map on the thread running the event loop. epoll_ctl() is thread-safe so the previous implementation needed no locking; the map does, and it is guarded accordingly. The lock is never held across a call into a handler, which is free to register or unregister descriptors. This only makes a handler safe against being unregistered by another handler in the same batch, on the thread running the loop. A handler destroyed by a different thread while the loop is between the lookup and the call was racy before and still is. The new test in tests/events/evtsource.cpp (which existed since many years but was completely empty) covers the fixed case without depending on timing: two pipes that are both readable are collected in one batch, and the handler that runs first unregisters the other, which must then not be called. It fails before this change -- both handlers run -- and passes after. Found while investigating a crash in aMule, where wxWebRequest's curl backend destroys transfer event sources from inside curl callbacks that themselves run during Dispatch(). Reported and reduced by ngosang at https://github.com/amule-org/amule/issues/1136, which also has a real-world reproducer built on wxFileSystemWatcher; under ASan it faulted in wxEpollDispatcher::Dispatch() on 4 of 5 runs before this change and 0 of 8 after. Closes #26924.