[Webkit-unassigned] [Bug 22567] HashTable needs to be made usable from multiple threads.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 1 01:43:46 PST 2008


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


ap at webkit.org changed:

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




------- Comment #3 from ap at webkit.org  2008-12-01 01:43 PDT -------
(From update of attachment 25620)
 HashTableStats::~HashTableStats()
 {
> +    MutexLocker lock(hashTableStatsMutex());

Locking a mutex in static object destructor can cause deadlocks at shutdown if
any thread was killed during process shutdown while holding this lock.

>      printf("\nkhtml::HashTable statistics\n\n");

This is a good opportunity to rename khtml::HashTable to WTF::HashTable.

>  void HashTableStats::recordCollisionAtCount(int count)
>  {
> +    MutexLocker lock(hashTableStatsMutex());

This works correctly, but only because HashTableStats data members that are
used in this function are not used anywhere else (except for HashTableStats
dtor). It would help if these data members were re-ordered in HashTableStats to
form a single group with a comment explaining how they are used.

> +// Thread safety: HashTable is only thread-safe with respect to the lookup/contains methods.
> +// Any other method use (include any iterators) should have locks around them when used from multiple threads.

I do not believe that this is formally true - lookup is certainly not safe if
the table is being modified from another thread at the same time.

But also, this description is quite vague, because it is not clear whether it
talks about class-level of object-level thread safety. And it is misguiding,
because locking mutexes is not the only form of synchronization that can
guarantee safety.

I'm not sure how to best describe HashTable threading model. It is pretty
common for container objects to have class-level thread safety, and to have
constant methods work on constant objects without synchronization though.

> -        static int numAccesses;
> +        static volatile int numAccesses;

Is this change needed?


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



More information about the webkit-unassigned mailing list