[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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #44423|review?                     |review+
               Flag|                            |




--- Comment #8 from Darin Adler <darin at apple.com>  2009-12-07 13:13:02 PST ---
(From update of attachment 44423)
The only thing I don't like about this is the name of
disableThreadVerification. I would prefer a name that describes the threading
regime being used rather than a simple "disable" call. But since the number of
callers is extremely small at this time, it seems we can change this later.

I'd eventually like to see the cross-thread string use be turned on, not in the
allocation of the StringImpl, but at the call sites. I know the class is used
cross thread "everywhere", but I think eventually we could do that. There are
tons of strings where it would be good to have the main thread assertion work.

> +        // Start thread verification as soon as the ref count gets to 2.
> +        // The class gets created with a ref count of 1 and then passed
> +        // to another thread where to ref count get increased.  This
> +        // is a heuristic but it seems to always work and has helped
> +        // find some bugs.

We use one space after a period, nottwo.

> +    ThreadVerifier()
> +        : m_isActive(false)
> +        , m_isThreadVerificationDisabled(false)
> +        , m_isOwnedByMainThread(false)
> +        , m_owningThread(0)
> +    { }

We'd normally put the braces on separate lines.

> @@ -44,7 +44,7 @@ IconRecord::IconRecord(const String& url)
>      , m_stamp(0)
>      , m_dataSet(false)
>  {
> -
> +    disableThreadVerification();
>  }

I'd like to see a comment at all the disableThreadVerification call sites about
why this needs to be done.

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