Crash when using profile manager [@ mozilla::imagelib::DiscardTracker::ReloadTimeout ]

RESOLVED FIXED

Status

()

Core
ImageLib
--
critical
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: scenor, Assigned: smaug)

Tracking

({crash, dogfood, regression})

Trunk
x86
Linux
crash, dogfood, regression
Points:
---
Bug Flags:
in-testsuite ?
in-litmus ?

Firefox Tracking Flags

(blocking2.0 final+)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b5pre) Gecko/20100821 Minefield/4.0b5pre
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b5pre) Gecko/20100821 Minefield/4.0b5pre

If I start FF with the Profile-Manager, it causes a segmentation fault after choosing profileand clicking "Start Minefield"
If I then start FF again without Profile-Manager, it starts with the selected Profile.
Using the -P flag also works.

Reproducible: Always

Steps to Reproduce:
see above
Actual Results:  
see above


http://crash-stats.mozilla.com/report/index/bp-d8f259a6-c4b2-4b96-a6aa-a123f2100821
(Assignee)

Updated

8 years ago
Blocks: 478398
(Assignee)

Comment 1

8 years ago
Created attachment 468027 [details] [diff] [review]
patch

I think this should be enough.
Assignee: nobody → Olli.Pettay
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #468027 - Flags: review?(bobbyholley+bmo)

Comment 2

8 years ago
Mozilla/5.0 (X11; Linux i686; rv:2.0b5pre) Gecko/20100821 Minefield/4.0b5pre

I only started seeing this in today's build so I'm not sure if that bug is the right one to point at.

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2d6ead22831c&tochange=5c5c3bf8dfeb

bp-0e9d9648-83f2-41e0-b94e-794732100821

bp-82a4c9b4-4415-4356-9489-39f412100821

Editing profiles.ini and changing StartWithLastProfile=0 to StartWithLastProfile=1 (with Default=1 on the correct profile) skips the Profile Manager and avoids the crash. Minefield hasn't crashed so far after that (keeping image discarding enabled).
Severity: normal → critical
Keywords: crash, regression
Summary: Segmentation fault after using profile manager → Crash when using profile manager [@ mozilla::imagelib::DiscardTracker::ReloadTimeout ]

Comment 3

8 years ago
I guess there's no tests that bring up the profile manager.

Simply starting with the Profile Manager and then quitting causes this crash for me (no need to even load a profile).
blocking2.0: --- → ?
status2.0: --- → ?
Flags: in-testsuite?
Flags: in-litmus?
(Reporter)

Comment 4

8 years ago
(In reply to comment #3)
> I guess there's no tests that bring up the profile manager.
> 
> Simply starting with the Profile Manager and then quitting causes this crash
> for me (no need to even load a profile).

That's right for me too, just didn't test it.
Severity: critical → normal
blocking2.0: ? → ---
(Reporter)

Updated

8 years ago
blocking2.0: --- → ?
(Reporter)

Comment 5

8 years ago
(In reply to comment #1)
> Created attachment 468027 [details] [diff] [review]
> patch
> 
> I think this should be enough.

Works for me
(Reporter)

Updated

8 years ago
Severity: normal → critical
Comment on attachment 468027 [details] [diff] [review]
patch

>diff --git a/modules/libpr0n/src/DiscardTracker.cpp b/modules/libpr0n/src/DiscardTracker.cpp
>--- a/modules/libpr0n/src/DiscardTracker.cpp
>+++ b/modules/libpr0n/src/DiscardTracker.cpp
>-void
>+nsresult
> DiscardTracker::ReloadTimeout()
> {
>   nsresult rv;
> 
>   // read the timeout pref
>   PRInt32 discardTimeout;
>   nsCOMPtr<nsIPrefBranch2> branch = do_GetService(NS_PREFSERVICE_CONTRACTID);
>+  NS_ENSURE_STATE(branch);
>   rv = branch->GetIntPref(DISCARD_TIMEOUT_PREF, &discardTimeout);
> 
>-  // If we got something bogus, return
>+  // If we got something bogus, keep the old value and return.
>   if (!NS_SUCCEEDED(rv) || discardTimeout <= 0)
>-    return;
>+    return NS_OK;

I don't think it's worth causing a ruckus if something goes wrong with the prefbranch - we should just emit an NS_WARNING and return, which will make us use the default value (defined statically at the top of the file). So do that, leave DiscardTracker::ReloadTimeout() as a void method, and consequently there's no need to change DiscardTracker::Initialize().

r=bholley with that change.
Attachment #468027 - Flags: review?(bobbyholley+bmo) → review+
Assignee: Olli.Pettay → nobody
Component: General → ImageLib
Keywords: dogfood
Product: Firefox → Core
QA Contact: general → imagelib
Assignee: nobody → Olli.Pettay
blocking2.0: ? → final+
status2.0: ? → ---

Updated

8 years ago
Version: unspecified → Trunk
(Assignee)

Comment 7

8 years ago
If we can't access the prefservice, something is in a strange state, like
in this case where we're actually shutting down.
IMO, we shouldn't run DiscardTracker::Initialize() then.
(Assignee)

Comment 8

8 years ago
..but actually that shouldn't matter much.
I'll change the patch and push it.
(Assignee)

Comment 9

8 years ago
http://hg.mozilla.org/mozilla-central/rev/b6a15d2bdb14
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Duplicate of this bug: 589590

Updated

8 years ago
Duplicate of this bug: 589749
Crash Signature: [@ mozilla::imagelib::DiscardTracker::ReloadTimeout ]
You need to log in before you can comment on or make changes to this bug.