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

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Mark Toller <mark.toller at samsung.com>  2012-09-26 09:30:00 PST ---
(In reply to comment #7)
> (From update of attachment 165777 [details])
> I think that you still misunderstand the purpose of this assertion. It is there to ensure that caller has taken the lock, so pthread_mutex_lock should fail, and there is no need to unlock.

OK, if that is indeed the case then :

1) The comment "m_scavengeMutex should be held before accessing m_scavengeThreadActive." is misleading, it should be stating "the caller [of signalScavenger()] should ensure that m_scavengeMutex is locked prior to calling this method."

2) The ASSERT *will* currently fail as the only location which locks the mutex (other than this 'trylock') is the scavengerThread itself (TCMalloc_PageHeap::scavengerThread()) - so once the scavengerThread waits on the condvar (and the mutex is then unlocked), the next call to signalScavenger() *will* obtain the lock with the trylock and therefore trigger the ASSERT.

I'm not sure if your comment is based on a misunderstanding of the pthread_cond_wait(&m_scavengeCondition, &m_scavengeMutex); code - this atomically waits on the condvar and unlocks the mutex.

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