Don't call handlers unregistered during wxEpollDispatcher::Dispatch() (PR #26931)
got3nks
Backport of ddd1e72 and 44efecd to 3.2, as discussed in #26930.
Derived from master's final state rather than replaying the map version first, since that would only be replaced again in the same PR.
Adaptations
Three constructs are not available here, so:
| master | 3.2 |
|---|---|
std::vector<std::unique_ptr<Entry>>, std::make_unique |
std::vector<Entry *>, new / delete in the dtor |
Entry() = default; with a default member initialiser |
explicit Entry() : handler(NULL) { } |
fd >= wxSsize(m_entries) |
static_cast<size_t>(fd) >= m_entries.size() |
plus nullptr to NULL and override to wxOVERRIDE. Behaviour is otherwise identical, including both wxCHECK_RET()s in ForgetEntry().
One incidental difference: comparing as size_t also rejects a negative descriptor, which the signed wxSsize() comparison on master does not.
Verified
Linux/aarch64, GCC 15.2, ASan build of 3.2 at 81eae11 (wx-config --version 3.2.12), forced rebuild and relink on each side.
tests/events/evtsource.cpp |
reproducer, 200 runs | |
|---|---|---|
stock 3.2 |
fails, 2 assertions | 0 pass, 200 heap-use-after-free |
| this change | passes, 7 assertions | 200 pass, 0 failures |
Full test suite on the patched build: 321 test cases, all passed (1286896 assertions), no failures.
The reproducer is the standalone one from #26924, unchanged. tests/events/evtsource.cpp was an empty stub on this branch too, and tests/Makefile.in already lists it, so no build system change is needed.
Not tested on x86_64.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26931
Commit Summary
- 7bd7b23 Don't call handlers unregistered during wxEpollDispatcher::Dispatch()
File Changes
(3 files)
- M include/wx/unix/private/epolldispatcher.h (48)
- M src/unix/epolldispatcher.cpp (80)
- M tests/events/evtsource.cpp (140)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26931.patch
- https://github.com/wxWidgets/wxWidgets/pull/26931.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26931)Thanks, I've added this to #26774 which will be merged in 3.2 soon, so closing this one.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
got3nks
got3nks left a comment (wxWidgets/wxWidgets#26931)Thanks! Any rough timeline for #26774 landing, or for a 3.2.12 release?
We are about to release aMule, so it would help to know whether to wait or carry the commit as a patch in our packaging in the meantime.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26931)The PR will be merged in the next few days, I do it monthly. The release will happen together with 3.3.4, which is what makes its date uncertain... Hopefully October, maybe November? I definitely want to do it before the end of the year.
Good luck with aMule release!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.