[Webkit-unassigned] [Bug 31639] Add asserts to RefCounted to make sure ref/deref happens on the right thread.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=31639


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #44201|review?                     |review-
               Flag|                            |




--- Comment #4 from David Levin <levin at chromium.org>  2009-12-04 10:13:53 PST ---
(From update of attachment 44201)
> diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
> +        (WTF::ThreadVerifier::activate): Activates hecks. Called when ref count becomes above 2.

typo: hecks

> diff --git a/JavaScriptCore/runtime/Structure.cpp b/JavaScriptCore/runtime/Structure.cpp
> +    // Use of addressOfCount() in this calss prevents thread verification in RefCounted

typo: calss

> +    // since it modifies refcount via other means then ref()/deref().

s/then/than/

> diff --git a/JavaScriptCore/wtf/RefCounted.h b/JavaScriptCore/wtf/RefCounted.h
> +#ifndef NDEBUG
> +class ThreadVerifier {

This seems like it should be in its own file. (As a side benefit, the whole
file can be ifndef NDEBUG include the wtf/Threading.h include.)

> +#endif

#endif // NDEBUG
Since this is a long ifdef.


> +#ifndef NDEBUG
> +    void disableThreadVerification()
> +    {

Personally, I'd prefer moving the ifndef inside of the method. This would still
make the call a no-op when NDEBUG is defined, but every call site wouldn't need
a ifndef NDEBUG around it.

> +        m_threadVerifier.disableThreadVerification();
> +    }
> +#endif
> +

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.