Assertion failure at shutdown if console listener isn't unregistered: PR_ASSERT(lock->owner == 0);

ASSIGNED
Assigned to

Status

()

Core
XPCOM
--
critical
ASSIGNED
11 years ago
9 years ago

People

(Reporter: WeirdAl, Assigned: timeless)

Tracking

({assertion, crash, testcase})

Trunk
x86
Windows XP
assertion, crash, testcase
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ccbr])

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
Created attachment 264302 [details]
stack trace

I was writing a xpcshell testcase for bug 380169, when I stumbled upon this crash.  Apparently, if you fail to unregister a console listener for any reason (such as an exception being thrown and not caught), you will crash on shutdown.

Testcase coming up as an attachment in a few minutes.
Flags: in-testsuite?
(Reporter)

Comment 1

11 years ago
Created attachment 264304 [details] [diff] [review]
xpcshell testcase (will crash on shutdown!)
(Assignee)

Comment 2

11 years ago
violates API:
04 xpcom_core!nsProxyObjectManager::~nsProxyObjectManager(void)+0x31
Hold Monitor:
0b xpcom_core!nsProxyEventObject::Release(void)+0xa0

This has nothing to do w/ the error console, it's either a bug in POM/PEO or a bug in XPCOM Startup/Shutdown, both of which are the responsibility of XPCOM (well, it could be the fault of the Error Service, but that too is in XPCOM, the error console is some ui).  
Assignee: js-console → nobody
Component: Error Console → XPCOM
QA Contact: error-console → xpcom
(Assignee)

Comment 3

11 years ago
Created attachment 264453 [details] [diff] [review]
this seems to work
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #264453 - Flags: superreview?(darin.moz)
Attachment #264453 - Flags: review?(ajvincent)
(Reporter)

Comment 4

11 years ago
Comment on attachment 264453 [details] [diff] [review]
this seems to work

Why are you asking me to review?  I don't know this code and have never touched anything close to it.  I don't know what proxies mean in this context.  Never mind how simple the patch is, I don't think I'm an appropriate reviewer.
Attachment #264453 - Flags: review?(ajvincent) → review?(benjamin)
(Reporter)

Comment 5

11 years ago
I do not crash with timeless's patch, it does indeed fix things.

Comment 6

11 years ago
Comment on attachment 264453 [details] [diff] [review]
this seems to work

The call to mProxyObject->LockedRemove(this) *must* be run within the POM monitor! In this case you're releasing the monitor before calling the destructor, which solves your bug but introduces unlocked access to the POM data structures.
Attachment #264453 - Flags: superreview?(darin.moz)
Attachment #264453 - Flags: review?(benjamin)
Attachment #264453 - Flags: review-
Version: unspecified → Trunk
(Assignee)

Updated

10 years ago
Blocks: 400450
Whiteboard: [ccbr]
You need to log in before you can comment on or make changes to this bug.