Caps crash on cleanup [@ DomainPolicy::Drop][@ 0x7a6f6d5c]

RESOLVED FIXED

Status

()

Core
Security: CAPS
--
critical
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: mao, Assigned: mao)

Tracking

(5 keywords)

Trunk
crash, fixed-aviary1.0.7, fixed1.7.12, regression, topcrash+
Points:
---
Bug Flags:
blocking1.7.11 -
blocking1.7.12 +
blocking-aviary1.0.7 +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(2 attachments)

(Assignee)

Description

12 years ago
Previously masked by bug 217967, whose crash happened early, rather than being
deferred to cleanup, and likely caused by residual "old" code still not properly
handling the home-made DomainPolicy reference counting (e.g.
nsScriptSecurityManager destructor, brutally deleting mDefaultPolicy instead of
dropping it).
Now I'm going to sleep (2:20 A.M. in my timezone), and tomorrow I'll attach a
coffee smelling patch ;-)
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

12 years ago
Severity: major → critical
Summary: Caps crash on cleanup (DomainPolicy::Drop) → Caps crash on cleanup [@ DomainPolicy::Drop]
(Assignee)

Comment 1

12 years ago
Created attachment 189625 [details] [diff] [review]
More  consistent DomainPolicy lifecycle management
[Checked in: Comment 8]

Sorry to be so late, but I've no way been able to reproduce the crash in a
debug session.
Hence, I'm trying to remove the hypotetical cause (premature deletion dued to
non-generalized use of reference count), crossing my fingers and hoping that
this patch estinguishes crash reports as soon as it gets landed :)

1) Removed the most prominent illegal direct delete (mDefaultPolicy in
nsScriptSecurityManager detructor) - not sure it is the cause, though, because
as far as I can see nsScriptSecurityManager deletion happens (or should happen)
after all nsPrincipal objects have been already deleted.
2) Added mDefaultPolicy null assignment to avoid double dropping under
exceptional circumnstances (e.g. out of memory errors during InitPolicies() )
3) Added an assertion to track deletions happening with an illegal reference
count
4) Incidentally, edited contributors list (also in nsPrincipal) to reflect bug
217967 changes and save caillon from getting blamed for my sins ;)
Attachment #189625 - Flags: review?(caillon)
Comment on attachment 189625 [details] [diff] [review]
More  consistent DomainPolicy lifecycle management
[Checked in: Comment 8]

r=caillon, but its probably worth a look to see what else needs similar
patching.
Attachment #189625 - Flags: review?(caillon) → review+
(Assignee)

Comment 3

12 years ago
(In reply to comment #2)
> its probably worth a look to see what else needs similar
> patching.
 
That's why I added this line:

NS_ASSERTION(mRefCount == 0, "Wrong refcount in DomainPolicy dtor");
 
I've been browsing since yesterday watching this assertion, and nothing violated
it so far...
(Assignee)

Updated

12 years ago
Attachment #189625 - Flags: superreview?(shaver)
(Assignee)

Updated

12 years ago
Attachment #189625 - Flags: superreview?(shaver) → superreview?(dveditz)
Comment on attachment 189625 [details] [diff] [review]
More  consistent DomainPolicy lifecycle management
[Checked in: Comment 8]

sr=dveditz
Attachment #189625 - Flags: superreview?(dveditz) → superreview+
(Assignee)

Updated

12 years ago
Attachment #189625 - Flags: approval1.8b4?
Attachment #189625 - Flags: approval1.7.10?
Attachment #189625 - Flags: approval-aviary1.0.6?
Attachment #189625 - Flags: approval1.8b4? → approval1.8b4+

Updated

12 years ago
Keywords: crash
Attachment #189625 - Flags: approval1.7.11?
Attachment #189625 - Flags: approval1.7.10?
Attachment #189625 - Flags: approval-aviary1.0.7?
Attachment #189625 - Flags: approval-aviary1.0.6?
+(K) Regression:
In v1.7.x, I've seen/crashed it for the first time after installing v1.7.10
(assumed fine up to v1.7.8);
In MAS-SM Trunk, I had seen it 2-3 times in +/- recent nightlies ("never" before).
Flags: blocking1.7.11?
Keywords: regression

Comment 6

12 years ago
*** Bug 301898 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 7

12 years ago
Comment on attachment 189625 [details] [diff] [review]
More  consistent DomainPolicy lifecycle management
[Checked in: Comment 8]

I'm going to attach a branch backport in minutes
Attachment #189625 - Flags: approval1.7.11?
Attachment #189625 - Flags: approval-aviary1.0.7?
(Assignee)

Comment 8

12 years ago
Created attachment 190309 [details] [diff] [review]
Trivial backport to aviary1.0.1 branch

My patch in attachment #189625 [details] [diff] [review] has been landed by timeless on trunk (2005-07-19
14:55). 
Trunk builds have disappeared from talkback records for this bug since then, so
I assume the bug is fixed.
This is a trivial backport of attachment #189625 [details] [diff] [review] for Aviary1.0.1 branch.
Should I close this bug as FIXED just now, or rather wait for this patch to be
landed on branch?
Attachment #190309 - Flags: review?(caillon)
Attachment #190309 - Flags: approval1.7.11?
Attachment #190309 - Flags: approval-aviary1.0.7?
Attachment #189625 - Attachment description: More consistent DomainPolicy lifecycle management → More consistent DomainPolicy lifecycle management [Checked in: Comment 8]
(Assignee)

Updated

12 years ago
Flags: blocking-aviary1.0.7?
(Assignee)

Comment 9

12 years ago
Marking FIXED as per timeless kind explaination :)
Yet to be fixed in 1.7 branches.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 10

12 years ago
this is a guess. jay/mao: correct me if i'm wrong
Summary: Caps crash on cleanup [@ DomainPolicy::Drop] → Caps crash on cleanup [@ DomainPolicy::Drop][@ 0x7a6f6d5c]
Attachment #190309 - Flags: review?(caillon) → review+

Comment 11

12 years ago
*** Bug 302166 has been marked as a duplicate of this bug. ***

Comment 12

12 years ago
1.7.11 is not blocked by this.
Flags: blocking1.7.11? → blocking1.7.11-

Updated

12 years ago
Attachment #190309 - Flags: approval1.7.11? → approval1.7.11-

Comment 13

12 years ago
(In reply to comment #12)
> 1.7.11 is not blocked by this.

Why do you think that 1.7.11 is not blocked by this?
My opinion is that even 1.7.10 should not have been released with this annoying
crash on exit, this was the most frequent crash reported by talkback.

Updated

12 years ago
Flags: blocking1.7.12+
Comment on attachment 190309 [details] [diff] [review]
Trivial backport to aviary1.0.1 branch

(In reply to comment #12)
> 1.7.11 is not blocked by this.

Unexpected since the fix is at hand, but well ... let's have v1.7.12 yet :-/
Attachment #190309 - Flags: approval1.7.12?
*** Bug 304320 has been marked as a duplicate of this bug. ***

Comment 16

12 years ago
Adding topcrash+ keyword since this is a major regression since 1.7.10.  We
really should make sure this makes it into 1.7.12, as it makes up close to 40%
of all crashes for 1.7.11:
http://talkback-public.mozilla.org/reports/mozilla/M1711/M1711-topcrashers.html

Sorry this didn't make it into 1.7.11, it should have, and would have if I was
on top of this during that short release cycle.
Keywords: topcrash+
*** Bug 306986 has been marked as a duplicate of this bug. ***

Updated

12 years ago
Flags: blocking-aviary1.0.7?
Flags: blocking1.7.12?

Updated

12 years ago
Flags: blocking1.7.12?
Flags: blocking1.7.12+
Flags: blocking-aviary1.0.7?
Flags: blocking-aviary1.0.7+
Attachment #190309 - Flags: approval1.7.13?
Attachment #190309 - Flags: approval1.7.12+
Attachment #190309 - Flags: approval-aviary1.0.8?
Attachment #190309 - Flags: approval-aviary1.0.7+
attachment 190309 [details] [diff] [review] checked in to MOZILLA_1_7_BRANCH and AVIARY_1_0_1_2005124_BRANCH.
Flags: blocking1.7.13+
Flags: blocking-aviary1.0.8?
Keywords: fixed-aviary1.0.7, fixed1.7.12

Comment 19

12 years ago
Can we get a final verification?
Crash Signature: [@ DomainPolicy::Drop] [@ 0x7a6f6d5c]
You need to log in before you can comment on or make changes to this bug.