Last Comment Bug 855402 - Profile Manager crashes at least 1 in 10 times when deleting a profile directory
: Profile Manager crashes at least 1 in 10 times when deleting a profile directory
Status: RESOLVED FIXED
: crash, regression
Product: Core
Classification: Components
Component: XBL (show other bugs)
: 20 Branch
: All All
: -- critical (vote)
: mozilla23
Assigned To: mjh563
:
Mentors:
Depends on:
Blocks: 720991 815847
  Show dependency treegraph
 
Reported: 2013-03-27 11:49 PDT by mjh563
Modified: 2013-05-23 08:29 PDT (History)
11 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
affected
affected
affected
unaffected


Attachments
Patch that fixes the crashing (962 bytes, patch)
2013-04-10 11:07 PDT, mjh563
ehsan: review+
Details | Diff | Splinter Review

Description mjh563 2013-03-27 11:49:33 PDT
Recently I've noticed that the Profile Manager will either crash or hang at least 1 in 10 times when deleting a profile directory.

Steps to reproduce:

1. Start the Profile Manager
2. Create 10 - 15 new profiles using the Create Profile button
3. Start and close Firefox once using each profile
4. Now delete each of the created profiles using the Delete Profile button (choose the option to delete all files)

Expected results:

All profiles can be deleted.

Actual results:

About 1 in 10 times the Profile Manager will either hang or crash when the "Delete Files" button is pressed (doing it 15 times, it will almost certainly happen).

Example crash report ID:
bp-fca37432-9a39-4ec9-9c26-92e042130327
Comment 1 mjh563 2013-03-27 11:51:47 PDT
The regression range is

Last good nightly: 2013-01-04
First bad nightly: 2013-01-05

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=801ba75ac563&tochange=d8ca3e1c469e
Comment 2 Scoobidiver (away) 2013-03-27 12:58:18 PDT
It's already reported in bug 720991 that exists before the regression.
Comment 3 Scoobidiver (away) 2013-04-03 10:00:44 PDT
(In reply to mjh563 from comment #1)
> Pushlog:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=801ba75ac563&tochange=d8ca3e1c469e
I suspect bug 819845 or bug 825527 in that range.
Comment 4 :Ehsan Akhgari 2013-04-03 10:05:23 PDT
Does the profile manager use <img> elements?
Comment 5 Boris Zbarsky [:bz] 2013-04-03 10:17:34 PDT
> I suspect bug 819845 or bug 825527 in that range.

Based on what, if I might ask?

The stack in comment 0 is a null-deref in jemalloc while calling free.  That suggests memory corruption.  Sadly, lots of things can corrupt memory that landed in that range.... :(
Comment 6 Scoobidiver (away) 2013-04-03 13:52:47 PDT
(In reply to Boris Zbarsky (:bz) from comment #5)
> > I suspect bug 819845 or bug 825527 in that range.
> Based on what, if I might ask?
They are the only ones about binding.

> The stack in comment 0 is a null-deref in jemalloc while calling free.
But others are not. See bug 720991 comment 22.
Comment 7 Boris Zbarsky [:bz] 2013-04-03 17:32:43 PDT
Oh, wrong binding.  There's XBL bindings and then there's DOM bindings, and they are pretty unrelated.  ;)

> But others are not. See bug 720991 comment 22.

Indeed.  Still all null-derefs...

Can someone reproduce this reliably enough to try bisecting on inbound tinderbox builds, perhaps?
Comment 8 Alice0775 White 2013-04-04 09:53:19 PDT
Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2453fdb37bf4
Mozilla/5.0 (Windows NT 5.1; rv:20.0) Gecko/20130103 Firefox/20.0 ID:20130103190906
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/16c6a3cf8a77
Mozilla/5.0 (Windows NT 5.1; rv:20.0) Gecko/20130103 Firefox/20.0 ID:20130103191908
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=2453fdb37bf4&tochange=16c6a3cf8a77
Comment 9 Boris Zbarsky [:bz] 2013-04-04 10:01:57 PDT
Alice0775, thanks!
Comment 10 Alice0775 White 2013-04-04 14:07:08 PDT
In local build, 
Last Good: 2453fdb37bf4
First Bad: 1871463a0542

Triggered by:
  1871463a0542	Ehsan Akhgari — Bug 815847 - Part 1: Remove nsXULPrototypeCache::gStartupCache; r=bsmedberg There is no good reason why this variable should be used.
Comment 11 mjh563 2013-04-06 09:29:34 PDT
I can confirm it starts happening at changeset 1871463a0542.

http://hg.mozilla.org/mozilla-central/rev/1871463a0542
Comment 12 Boris Zbarsky [:bz] 2013-04-06 09:33:35 PDT
Perhaps the nsXULPrototypeCache::WritePrototype change there, which is failing to write things out or something?
Comment 13 :Ehsan Akhgari 2013-04-08 11:37:19 PDT
I have no clue what could have caused this...
Comment 14 mjh563 2013-04-09 14:09:35 PDT
I don't pretend to understand the code in any detail, but the crash seems to be caused by the removal of the if statement at lines 1.55-1.56 in the patch.

Should that check have been replaced with one like the following (similar to what's done elsewhere in the patch), rather than being removed completely?

StartupCache* sc = StartupCache::GetSingleton();
if (!sc)
    return NS_OK;

Building 1871463a0542 with this added code does stop it crashing for me, anyway.
Comment 15 Boris Zbarsky [:bz] 2013-04-09 19:01:43 PDT
That's what I was looking at in comment 12, too.

Mind just posting your patch?  ;)
Comment 16 mjh563 2013-04-10 11:07:14 PDT
Created attachment 735879 [details] [diff] [review]
Patch that fixes the crashing
Comment 17 :Ehsan Akhgari 2013-04-11 09:35:12 PDT
Comment on attachment 735879 [details] [diff] [review]
Patch that fixes the crashing

Review of attachment 735879 [details] [diff] [review]:
-----------------------------------------------------------------

If this fixes the crash, then r=me!
Comment 18 Boris Zbarsky [:bz] 2013-04-11 09:37:11 PDT
mjh563@yahoo.co.uk, can you push this yourself, or do you need someone to push it for you?
Comment 19 mjh563 2013-04-11 10:11:41 PDT
Could someone push it for me please. I assume I don't have the necessary access to do it myself. Thanks!
Comment 20 Boris Zbarsky [:bz] 2013-04-11 11:33:26 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/83b8038839f1

Thank you for putting that together and testing it out!
Comment 21 Ryan VanderMeulen [:RyanVM] 2013-04-12 05:13:23 PDT
https://hg.mozilla.org/mozilla-central/rev/83b8038839f1
Comment 22 Mike Kaply [:mkaply] 2013-05-23 06:59:41 PDT
Should 720991 be marked fixed as well? Also, should this go in the Firefox 17 ESR?
Comment 23 Scoobidiver (away) 2013-05-23 08:29:17 PDT
(In reply to Michael Kaply (mkaply) from comment #22)
> Should 720991 be marked fixed as well?
No, because of bp-6db665ec-5dd7-49e0-8852-3e94c2130521 and many others.

> Also, should this go in the Firefox 17 ESR?
It's unaffected.

Note You need to log in before you can comment on or make changes to this bug.