Title: [248544] trunk/Source/WTF
- Revision
- 248544
- Author
- [email protected]
- Date
- 2019-08-12 13:15:26 -0700 (Mon, 12 Aug 2019)
Log Message
Unreviewed, fix post landing review comments for r248533 from Darin.
* wtf/RefCounted.h:
(WTF::RefCountedBase::ref const):
(WTF::RefCountedBase::applyRefDerefThreadingCheck const):
(WTF::RefCountedBase::derefBase const):
(WTF::RefCountedBase::areThreadingCheckedEnabled const): Deleted.
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (248543 => 248544)
--- trunk/Source/WTF/ChangeLog 2019-08-12 19:46:40 UTC (rev 248543)
+++ trunk/Source/WTF/ChangeLog 2019-08-12 20:15:26 UTC (rev 248544)
@@ -1,5 +1,15 @@
2019-08-12 Chris Dumez <[email protected]>
+ Unreviewed, fix post landing review comments for r248533 from Darin.
+
+ * wtf/RefCounted.h:
+ (WTF::RefCountedBase::ref const):
+ (WTF::RefCountedBase::applyRefDerefThreadingCheck const):
+ (WTF::RefCountedBase::derefBase const):
+ (WTF::RefCountedBase::areThreadingCheckedEnabled const): Deleted.
+
+2019-08-12 Chris Dumez <[email protected]>
+
Add threading assertions to RefCounted
https://bugs.webkit.org/show_bug.cgi?id=200507
Modified: trunk/Source/WTF/wtf/RefCounted.h (248543 => 248544)
--- trunk/Source/WTF/wtf/RefCounted.h 2019-08-12 19:46:40 UTC (rev 248543)
+++ trunk/Source/WTF/wtf/RefCounted.h 2019-08-12 20:15:26 UTC (rev 248544)
@@ -40,16 +40,8 @@
public:
void ref() const
{
-#if !ASSERT_DISABLED
- if (m_isOwnedByMainThread != isMainThread() && hasOneRef())
- m_isOwnedByMainThread = isMainThread(); // Likely ownership transfer.
+ applyRefDerefThreadingCheck();
- // If you hit this assertion, it means that the RefCounted object was ref'd or deref'd
- // concurrent from several threads, which is not safe. You should either subclass
- // ThreadSafeRefCounted instead, or make sure to always ref / deref from the same thread.
- ASSERT_WITH_MESSAGE(!areThreadingCheckedEnabled() || m_isOwnedByMainThread == isMainThread(), "Should not be ref'd / deref'd concurrently from several threads");
-#endif
-
#if CHECK_REF_COUNTED_LIFECYCLE
ASSERT_WITH_SECURITY_IMPLICATION(!m_deletionHasBegun);
ASSERT(!m_adoptionIsRequired);
@@ -108,12 +100,21 @@
{
}
+ void applyRefDerefThreadingCheck() const
+ {
#if !ASSERT_DISABLED
- bool areThreadingCheckedEnabled() const
- {
- return areThreadingChecksEnabledGlobally && m_areThreadingChecksEnabled;
+ if (hasOneRef()) {
+ // Likely an ownership transfer across threads that may be safe.
+ m_isOwnedByMainThread = isMainThread();
+ } else if (areThreadingChecksEnabledGlobally && m_areThreadingChecksEnabled) {
+ // If you hit this assertion, it means that the RefCounted object was ref/deref'd
+ // from both the main thread and another in a way that is likely concurrent and unsafe.
+ // Derive from ThreadSafeRefCounted and make sure the destructor is safe on threads
+ // that call deref, or ref/deref from a single thread.
+ ASSERT_WITH_MESSAGE(m_isOwnedByMainThread == isMainThread(), "Unsafe to ref/deref from different threads");
+ }
+#endif
}
-#endif
~RefCountedBase()
{
@@ -126,16 +127,8 @@
// Returns whether the pointer should be freed or not.
bool derefBase() const
{
-#if !ASSERT_DISABLED
- if (m_isOwnedByMainThread != isMainThread() && hasOneRef())
- m_isOwnedByMainThread = isMainThread(); // Likely ownership transfer.
+ applyRefDerefThreadingCheck();
- // If you hit this assertion, it means that the RefCounted object was ref'd or deref'd
- // concurrent from several threads, which is not safe. You should either subclass
- // ThreadSafeRefCounted instead, or make sure to always ref / deref from the same thread.
- ASSERT_WITH_MESSAGE(!areThreadingCheckedEnabled() || m_isOwnedByMainThread == isMainThread(), "Should not be ref'd / deref'd concurrently from several threads");
-#endif
-
#if CHECK_REF_COUNTED_LIFECYCLE
ASSERT_WITH_SECURITY_IMPLICATION(!m_deletionHasBegun);
ASSERT(!m_adoptionIsRequired);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
