Closed
Bug 998706
Opened 10 years ago
Closed 10 years ago
dom/workers/MessagePort.cpp : ASSERTION: bad size recorded: 'aInstanceSize == 0 || entry->GetClassSize() == aInstanceSize
Categories
(Core :: DOM: Workers, defect)
Core
DOM: Workers
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: ishikawa, Assigned: ishikawa)
Details
Attachments
(1 file, 1 obsolete file)
1.08 KB,
patch
|
bent.mozilla
:
review+
|
Details | Diff | Splinter Review |
Quoted from dev-platform mailing list. I see the following ASSERTION error about *800* times during test run of |mach mochitest-plain| of full debug build of FF created from M-C portion (./mozilla) of C-C source tree. They look grave but test harness seems to pass the tests themsevles most of the time. ###!!! ASSERTION: bad size recorded: 'aInstanceSize == 0 || entry->GetClassSize() == aInstanceSize', file /REF-COMM-CENTRAL/comm-central/mozilla/xpcom/base/nsTraceRefcnt.cpp, line 441 What does this error mean? I have not seen these assertions in full debug TB test and so not sure how grave or light the assertions are. A typical trace shows something like this (sanitized a bit to remove the timestamp at the beginning of the each line) : [Parent 19583] ###!!! ASSERTION: bad size recorded: 'aInstanceSize == 0 || entry->GetClassSize() == aInstanceSize', file /REF-COMM-CENTRAL/comm-central/mozilla/xpcom/base/nsTraceRefcnt.cpp, line 441 NS_LogAddRef (/REF-COMM-CENTRAL/comm-central/mozilla/xpcom/base/nsTraceRefcnt.cpp:979) mozilla::dom::workers::MessagePort::AddRef() (/REF-COMM-CENTRAL/comm-central/mozilla/dom/workers/MessagePort.cpp:220) mozilla::dom::workers::SharedWorker::SharedWorker(nsPIDOMWindow*, mozilla::dom::workers::WorkerPrivate*) (/WWW-DIR/ff-objdir-tb3/dom/workers/../../dist/include/nsAutoPtr.h:879) mozilla::dom::workers::RuntimeService::CreateSharedWorker(mozilla::dom::GlobalObject const&, nsAString_internal const&, nsACString_internal const&, mozilla::dom::workers::SharedWorker**) (/WWW-DIR/ff-objdir-tb3/dom/workers/../../dist/include/nsAutoPtr.h:917) ... NOTE: Interesting thing is that almost all such ASSERTIONS have the following line at the top of the trace. (I think "ALL" is likely, but I may have missed a few lines.) mozilla::dom::workers::MessagePort::AddRef() (/REF-COMM-CENTRAL/comm-central/mozilla/dom/workers/MessagePort.cpp:220) --- To which David Baron responded as follows. It probably means that there are two different classes reporting the same name to nsTraceRefcnt. Classes that use the NS_IMPL_ISUPPORTSn or NS_IMPL_ADDREF + NS_IMPL_RELEASE macros should use the fully qualified class name and not depend on being inside namespace declarations. In this case, I think the problem is mozilla::dom::MessagePort and mozilla::dom::workers::MessagePort. -David Summary: Indeed, when I applied the attached patch (I used a full qualification starting from mozilla:: ...), the assertion disappeared! TIA
Assignee | ||
Updated•10 years ago
|
Attachment #8409348 -
Flags: review?(bent.mozilla)
Comment 1•10 years ago
|
||
Comment on attachment 8409348 [details] [diff] [review] A patch : Use the full namescope qualification. Although the assertion only fires for ADDREF, I think you need to change both ADDREF and RELEASE because the logging expects the same class name for both.
Assignee | ||
Comment 2•10 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #1) > Comment on attachment 8409348 [details] [diff] [review] > A patch : Use the full namescope qualification. > > Although the assertion only fires for ADDREF, I think you need to change > both ADDREF and RELEASE because the logging expects the same class name for > both. Of course. But strange thing is that there were no errors/warnings when I only modified ADDREF during test. Anyway, the attached new patch changes RELEASE part as well. The new patch does not produce any warnings/errors either during local mochitest-plain test. So this patch should be OK. TIA
Attachment #8409448 -
Flags: review?(bent.mozilla)
Comment on attachment 8409448 [details] [diff] [review] A patch (take 2) : Use the full namescope qualification. Thanks!
Attachment #8409448 -
Flags: review?(bent.mozilla) → review+
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to ben turner [:bent] (use the needinfo? flag!) from comment #3) > Comment on attachment 8409448 [details] [diff] [review] > A patch (take 2) : Use the full namescope qualification. > > Thanks! You are welcome and I put checkin-needed keyword. Thank you.
Keywords: checkin-needed
Updated•10 years ago
|
Attachment #8409348 -
Attachment is obsolete: true
Attachment #8409348 -
Flags: review?(bent.mozilla)
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/57da6f0d047e
Assignee: nobody → ishikawa
Keywords: checkin-needed
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/57da6f0d047e
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•