Closed
Bug 1079311
Opened 10 years ago
Closed 10 years ago
Attempt to lock an already-locked mutex in nsComponentManangerImpl::RegisterContractIDLocked
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: jdm, Assigned: bmarsd, Mentored)
Details
(Whiteboard: [good first bug][lang=c++])
Attachments
(2 files, 1 obsolete file)
3.42 KB,
text/plain
|
Details | |
2.54 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
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•10 years ago
|
||
See bug 881237 for the RegisterCIDEntryLocked fix and a good starting place for the fix and how to write a testcase.
Assignee | ||
Comment 2•10 years ago
|
||
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)
Reporter | ||
Comment 3•10 years ago
|
||
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+
Assignee | ||
Comment 4•10 years ago
|
||
(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?
Reporter | ||
Comment 5•10 years ago
|
||
Ah, yes. Feel free to change that to an NS_WARNING.
Assignee | ||
Comment 6•10 years ago
|
||
Changed NS_ERROR to NS_WARNING to avoid crash in debug builds.
Attachment #8501516 -
Attachment is obsolete: true
Attachment #8502129 -
Flags: review?(benjamin)
Updated•10 years ago
|
Attachment #8502129 -
Flags: review?(benjamin) → review+
Reporter | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/64aa0ab1f87b
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → bmarsd
https://hg.mozilla.org/mozilla-central/rev/64aa0ab1f87b
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•