Closed Bug 1079311 Opened 5 years ago Closed 5 years ago

Attempt to lock an already-locked mutex in nsComponentManangerImpl::RegisterContractIDLocked


(Core :: XPCOM, defect)

Not set





(Reporter: jdm, Assigned: bmarsd, Mentored)


(Whiteboard: [good first bug][lang=c++])


(2 files, 1 obsolete file)

Attached file Backtrace
In RegisterCIDEntryLocked we correctly unlock the mutex before calling LogMessage, but in RegisterContractIDLocked we don't. This causes problems when trying to report errors.
See bug 881237 for the RegisterCIDEntryLocked fix and a good starting place for the fix and how to write a testcase.
I'm not sure if I'm on the right track with the test. With my debug build, NS_ERROR() crashes the test before it even gets to LogMessage(). If I remove the NS_ERROR(), the test seems to work as expected.

Testing with `mach xpcshell-test xpcom/tests/unit/test_compmgr_warnings.js`.
Attachment #8501516 - Flags: feedback?(josh)
Comment on attachment 8501516 [details] [diff] [review]
Unlock mutex before logging error in RegisterContractIDLocked

Review of attachment 8501516 [details] [diff] [review]:

This looks correct to me. Which NS_ERROR are you referring to?
Attachment #8501516 - Flags: feedback?(josh) → feedback+
(In reply to Josh Matthews [:jdm] from comment #3)
> This looks correct to me. Which NS_ERROR are you referring to?

In RegisterContractIDLocked, on line 566:
    NS_ERROR("No CID found when attempting to map contract ID");

[11280] ###!!! ASSERTION: No CID found when attempting to map contract ID: 'Error', file /home/brian/Downloads/mozilla-central/xpcom/components/nsComponentManager.cpp, line 566
Hit MOZ_CRASH() at /home/brian/Downloads/mozilla-central/memory/mozalloc/mozalloc_abort.cpp:37
TEST-UNEXPECTED-FAIL | /home/brian/Downloads/mozilla-central/obj-x86_64-unknown-linux-gnu/_tests/xpcshell/xpcom/tests/unit/test_compmgr_warnings.js | test failed (with xpcshell return code: -11)

The test is trying to check that LogMessage doesn't crash, but NS_ERROR is always called before LogMessage, so it crashes regardless. Maybe it should be an NS_WARNING instead, like in RegisterCIDEntryLocked?
Ah, yes. Feel free to change that to an NS_WARNING.
Changed NS_ERROR to NS_WARNING to avoid crash in debug builds.
Attachment #8501516 - Attachment is obsolete: true
Attachment #8502129 - Flags: review?(benjamin)
Attachment #8502129 - Flags: review?(benjamin) → review+
Assignee: nobody → bmarsd
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.