Closed Bug 66494 Opened 24 years ago Closed 24 years ago

Assertion in nsAutoLock.cpp

Categories

(Core :: XPCOM, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mcafee, Assigned: darin.moz)

Details

Attachments

(2 files)

I am seeing pages of this assertion after landing the pierre backout tonight: ###!!! ASSERTION: potential deadlock between Lock@86f3b88 and Monitor@86d1578: 'Error', file nsAutoLock.cpp, line 250
This is a result of my changes. I talked to brendan about this a while back. I concluded that these warnings are erroneous b/c nsAutoLock does not properly cleanup after itself. Brendan pointed me to a bug that he's working on that will hopefully solve this.
These assertions showed up in a number of the tinderbox bloat tests right before the test hung while loading ftp://ftp.mozilla.org/ (look at the orange harpoon and shrike builds). I don't see the assertions in the green builds that I've looked at. Is there a chance there's real deadlock here, or is it just that the assertions are an incorrect result of something being hung?
Ok... it's interesting that we are hanging on ftp.mozilla.org on those few systems. It does sound like the deadlock is being met. It would be contention between nsStreamListenerProxy's mLock and nsPipe's mMonitor that we're observing. I have another solution to nsStreamListenerProxy's design that will enable me to remove the nesting of these two locks. Brendan warned me about using nested locks... but I didn't see how we could possibly reach a deadlock... seems I might have been mistaken. I will work on the patch to remove the nesting of these locks.
The PRLock pointer replay problem in nsAutoLock's deadlock detection is a bug (covered somewhat implicitly in bug 58904), but I would be surprised if it bit reproducibly, given all the random UI event driven allocations. Years ago I warned chouck, who was hacking on JS in its own thread, about nested locks inviting deadlock and contention, and he said "deadlock, schmedlock!". I love playing Cassandra. On to the patch: else after continue is a non-sequitur that over-indents the subsequent code. There's a pre-existing else after return a few lines later. + continue; + } + else { Looks good otherwise. There's almost always a way to be "leafy" and avoid nesting. r/sr=brendan@mozilla.org if it helps. /be
over to darin, looks like he's got a fix ready.
Assignee: scc → darin
Patch checked into trunk. Please reopen if you see the ASSERTION again on tinderbox.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: