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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: ishikawa, Assigned: ishikawa)

Details

Attachments

(1 file, 1 obsolete file)

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
Attachment #8409348 - Flags: review?(bent.mozilla)
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.
(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+
(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
Attachment #8409348 - Attachment is obsolete: true
Attachment #8409348 - Flags: review?(bent.mozilla)
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.