[Webkit-unassigned] [Bug 97539] New: Broken and incorrect code in FastMalloc.cpp

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Sep 25 01:53:58 PDT 2012


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

           Summary: Broken and incorrect code in FastMalloc.cpp
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: PC
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: Minor
          Priority: P2
         Component: WebCore Misc.
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: mark.toller at samsung.com


In the pthread supported #ifdef branch in Source/WTF/wtf/FastMalloc.cpp, the method TCMalloc_PageHeap::signalScavenger() contains the following code:

    // m_scavengeMutex should be held before accessing m_scavengeThreadActive.
    ASSERT(pthread_mutex_trylock(m_scavengeMutex));
    if (!m_scavengeThreadActive && shouldScavenge())
        pthread_cond_signal(&m_scavengeCondition);

There are a couple of problems with this:

1) The TCMalloc code can't be compiled in (without changing the build system/editing the file) if debug is enabled, and the ASSERT() macro is compiled out if DEBUG is not enabled. This means that the 'trylock' code is never compiled.
2) The pthread_mutex_trylock(m_scavengeMutex) code won't compile - as it requires a pointer to the mutex (pthread_mutex_trylock(&m_scavengeMutex);)
3) If the (fixed) code was compiled in, it will either:
    a) cause a deadlock if the mutex is locked successfully, as it is not unlocked afterwards.
    b) cause the ASSERT to fire if the mutex is not taken (which can happen).

The comment indicates that we should take the mutex before checking m_scavengeThreadActive. However, this will cause us to perform a lock/unlock on the mutex for many delete operations.  The worst case scenario for *not* locking the mutex is that we either :
     1) signal the condvar more often than required when we get several deletes before the m_scavengeThreadActive is set to true or ....
     2) We don't signal the condvar as early as possible, and wait till the next delete...

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



More information about the webkit-unassigned mailing list