Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#6386 closed defect (fixed)

Deadlock/hang/crash on TBB New Identity

Reported by: mikeperry Owned by: mikeperry
Priority: High Milestone:
Component: Firefox Patch Issues Version:
Severity: Keywords: MikePerry201303
Cc: Actual Points: 3
Parent ID: Points:
Reviewer: Sponsor:

Description

Recently, I've noticed occasional deadlock on New Identity, especially if the browser is busy doing network activity when it is clicked, but not always.

I have not yet established a solid repro case in order to debug it.

Child Tickets

Change History (9)

comment:1 Changed 7 years ago by cypherpunks

I can confirm this sort of freezing.

To reproduce this bug, you can try some Wordpress Blogs which are often overloaded with plugins and other external content.

comment:2 Changed 7 years ago by mikeperry

I actually just survived the deadlock here by clicking "Stop Script" on a "This script has stopped responding" warning. Firefox claimed that the offending line was https://gitweb.torproject.org/torbutton.git/blob/master:/src/chrome/content/torbutton.js#l2304, which is a call to remove[i].contentWindow.close() in torbutton_close_on_toggle().

However, in all other cases, I never even got the unresponsive script warning, so who knows if this is the root issue/only issue...

comment:3 Changed 7 years ago by cypherpunks

Unchanged situation. There is still this annoying deadlock on "New Identity", occuring each time a web page overloaded with external content is loaded.

Mike, any progress in this issue?

comment:4 in reply to:  3 Changed 7 years ago by mikeperry

Replying to cypherpunks:

Unchanged situation. There is still this annoying deadlock on "New Identity", occuring each time a web page overloaded with external content is loaded.

Mike, any progress in this issue?

No, it actually hasn't happened for me in quite a while. The closest I've come was a rather long delay after hitting "New Identity" a few times in a row. The browser did recover in that case, though, so it could have been something else churning CPU (possibly that window close call from my earlier comment, which may actually be a different bug).

If you think the external content/plugins/popups are related, a test html page or URL that can at least semi-reliably generate the deadlock would go a long way towards helping to figure it out.

comment:5 Changed 7 years ago by mikeperry

Summary: Deadlock on TBB New IdentityDeadlock/hang on TBB New Identity

Just hit this again with FF17. Unfortunately it was on a TBB build without symbols. gdb said all threads were blocked on a pthread_cont_wait, so it could be due to our cache hack patch for #5715. I think there is now a better cache API we could try to use instead (#7637), but it's not clear if that API is sufficient to prevent the race from #5715 in all cases.

In the meantime, I will pepper New Identity with Torbutton loglines at INFO level in case we get a repro. It could still be something else, too :/.

comment:6 Changed 7 years ago by mikeperry

Aha: I found https://hg.mozilla.org/mozilla-central/rev/38fcd9047d39 while digging through the cache service code. Apparently, it was added to the cache shutdown syncing code because of a crash bug: https://bugzilla.mozilla.org/show_bug.cgi?id=673543. However, looking at the codepaths, the fix for that crash would *also* prevent the release of our patch condition's lock by nsCacheService::ProcessRequest(). That code releases the lock we were holding and then waits for *another* condition using the same lock. This could definitely cause our condition to never get properly signaled if the timing was just right, causing deadlock.

So it's possible that their crash fix will fix this issue too. I'm going to apply it to our #5715 patch and I guess we'll just wait another couple months to see if this race resurfaces, at which point I suppose I could dig deeper into #7637. But at a glance, #7637 will require way more mechanics than this simple fix, and it might re-introduce the evercookie race. So here's hoping this is it!

comment:7 Changed 7 years ago by mikeperry

Resolution: fixed
Status: newclosed
Summary: Deadlock/hang on TBB New IdentityDeadlock/hang/crash on TBB New Identity

Ok, I am confident that the above patch will at least solve *some* instances of hangs and crashes, but more may remain. If they still happen in subsequent TBB releases, please file a new bug.

This fix will appear in the next TBB-stable and -alpha releases.

comment:8 Changed 7 years ago by mikeperry

Actual Points: 3
Keywords: MikePerry201303 added

comment:9 Changed 7 years ago by mikeperry

Sadly, of course more issues remain. See #8559.

Note: See TracTickets for help on using tickets.