Closed Bug 1079311 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: XPCOM, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: jdm, Assigned: bmarsd, Mentored)

Details

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

Attachments

(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.

http://mxr.mozilla.org/mozilla-central/source/xpcom/components/nsComponentManager.cpp#571
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
https://hg.mozilla.org/mozilla-central/rev/64aa0ab1f87b
Status: NEW → RESOLVED
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.