Made wxObjectRefData reference counting thread-safe (PR #27042)
Iolanda Asavei
This fixes a data race in wxObjectRefData reference counting when
wxGDIRefData-based objects are used concurrently from multiple threads.
The issue became reproducible after the wxGCDCImpl::DoDrawPoint()
changes in 4ed8ccf (#19037), which introduced wxDCPenChanger and
wxDCBrushChanger and consequently copies of the shared stock pen and
brush.
ThreadSanitizer reports races in wxRefCounter::IncRef()/DecRef() between
worker threads and the main GUI thread. These can eventually result in a
heap-use-after-free crash, typically observed in an unrelated
wxGDIObject::IsOk() call.
The fix changes wxObjectRefData to use the existing wxRefCounterMT and
moves refcountermt.h out of wx/private, as it is now included from the
public wx/object.h.
I've attached a small sample reproducing the issue. With an affected
build it triggers the race/crash when multiple threads draw points on
their own wxMemoryDCs. With this change, the sample runs without TSan
reporting the reference-counting races.
WARNING: ThreadSanitizer: data race
Write of size 4 by main thread:
#0 wxRefCounter::IncRef() .../include/wx/object.h:235
#1 wxObject::Ref(wxObject const&)
...
Previous write of size 4 by thread T12:
#0 wxRefCounter::IncRef() .../include/wx/object.h:235
...
#12 wxDCBrushChanger::wxDCBrushChanger(...)
#13 wxGCDCImpl::DoDrawPoint(int, int)
#14 wxDC::DrawPoint(int, int)
#15 WorkerThreadProc ...
SUMMARY: ThreadSanitizer: data race .../include/wx/object.h:235 in wxRefCounter::IncRef()
```WARNING: ThreadSanitizer: data race
Write of size 4 by main thread:
#0 wxRefCounter::IncRef() .../include/wx/object.h:235
#1 wxObject::Ref(wxObject const&)
...
Previous write of size 4 by thread T12:
#0 wxRefCounter::IncRef() .../include/wx/object.h:235
...
#12 wxDCBrushChanger::wxDCBrushChanger(...)
#13 wxGCDCImpl::DoDrawPoint(int, int)
#14 wxDC::DrawPoint(int, int)
#15 WorkerThreadProc ...
SUMMARY: ThreadSanitizer: data race .../include/wx/object.h:235 in wxRefCounter::IncRef()
`
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/27042
Commit Summary
- b3ac0b5 Made wxObjectRefData reference counting thread-safe
File Changes
(9 files)
- M build/bakefiles/files.bkl (1)
- M build/cmake/files.cmake (1)
- M build/files (1)
- M build/msw/wx_base.vcxproj (1)
- M build/msw/wx_base.vcxproj.filters (3)
- M include/wx/object.h (3)
- M include/wx/private/animate.h (4)
- M include/wx/private/webrequest.h (2)
- R include/wx/refcountermt.h (12)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/27042.patch
- https://github.com/wxWidgets/wxWidgets/pull/27042.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.
Iolanda Asavei
@iolandaasavei pushed 1 commit.
- 4b13ab7 Added refcountermt.h to allheaders.h
—
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#27042)GUI objects can only be used from the main thread, any program using them from another thread is malformed. Atomic ref counting doesn't help enough so I don't think this should be merged.
—
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.