Profile Manager crashes at least 1 in 10 times when deleting a profile directory

RESOLVED FIXED in mozilla23

Status

()

Core
XBL
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mjh563, Assigned: mjh563)

Tracking

({crash, regression})

20 Branch
mozilla23
crash, regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox19 unaffected, firefox20 affected, firefox21 affected, firefox22 affected, firefox-esr17 unaffected)

Details

(crash signature)

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
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
(Assignee)

Comment 1

4 years ago
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

4 years ago
It's already reported in bug 720991 that exists before the regression.
Blocks: 720991
Severity: normal → critical
Status: UNCONFIRMED → NEW
Crash Signature: [@ jemalloc_crash | free | moz_free | nsACString_internal::Finalize() ] [@ nsXBLService::GetBinding(nsIContent*, nsIURI*, bool, nsIPrincipal*, bool*, nsXBLBinding**, nsTArray<nsIURI*>&) ]
status-firefox19: --- → unaffected
status-firefox20: --- → affected
status-firefox21: --- → affected
status-firefox22: --- → affected
Component: Untriaged → XBL
Ever confirmed: true
Keywords: crash, regression
OS: Linux → All
Product: Firefox → Core
Hardware: x86 → All

Comment 3

4 years ago
(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.
Does the profile manager use <img> elements?
> 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

4 years ago
(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.
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?

Updated

4 years ago
Keywords: regressionwindow-wanted

Comment 8

4 years ago
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
Alice0775, thanks!
Blocks: 815847
Flags: needinfo?(ehsan)
Keywords: regressionwindow-wanted

Comment 10

4 years ago
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.
(Assignee)

Comment 11

4 years ago
I can confirm it starts happening at changeset 1871463a0542.

http://hg.mozilla.org/mozilla-central/rev/1871463a0542
Perhaps the nsXULPrototypeCache::WritePrototype change there, which is failing to write things out or something?
I have no clue what could have caused this...
Flags: needinfo?(ehsan)
(Assignee)

Comment 14

4 years ago
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.
That's what I was looking at in comment 12, too.

Mind just posting your patch?  ;)
(Assignee)

Comment 16

4 years ago
Created attachment 735879 [details] [diff] [review]
Patch that fixes the crashing

Updated

4 years ago
Attachment #735879 - Flags: review?(ehsan)
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!
Attachment #735879 - Flags: review?(ehsan) → review+
mjh563@yahoo.co.uk, can you push this yourself, or do you need someone to push it for you?
Flags: needinfo?(mjh563)
(Assignee)

Comment 19

4 years ago
Could someone push it for me please. I assume I don't have the necessary access to do it myself. Thanks!
Flags: needinfo?(mjh563)
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/83b8038839f1

Thank you for putting that together and testing it out!
Assignee: nobody → mjh563
Flags: in-testsuite?
Keywords: checkin-needed
Target Milestone: --- → mozilla23

Updated

4 years ago
Keywords: checkin-needed

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/83b8038839f1
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Should 720991 be marked fixed as well? Also, should this go in the Firefox 17 ESR?

Comment 23

4 years ago
(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.
status-firefox-esr17: --- → unaffected
You need to log in before you can comment on or make changes to this bug.