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

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Josh Matthews [:jdm]]

Attachment: 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

Comment 1

[Benjamin Smedberg]

See bug 881237 for the RegisterCIDEntryLocked fix and a good starting place for the fix and how to write a testcase.

Comment 2

[Brian Marshall]

Attachment: Unlock mutex before logging error in RegisterContractIDLocked

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

Comment 3

[Josh Matthews [:jdm]]

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?

Comment 4

[Brian Marshall]

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

Comment 5

[Josh Matthews [:jdm]]

Ah, yes. Feel free to change that to an NS_WARNING.

Comment 6

[Brian Marshall]

Attachment: Unlock mutex before logging error in RegisterContractIDLocked

Changed NS_ERROR to NS_WARNING to avoid crash in debug builds.

Comment 7

[Josh Matthews [:jdm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/64aa0ab1f87b

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/64aa0ab1f87b