998706 - dom/workers/MessagePort.cpp : ASSERTION: bad size recorded: 'aInstanceSize == 0 || entry->GetClassSize() == aInstanceSize

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect)

Description

[ISHIKAWA, Chiaki]

Attachment: A patch : Use the full namescope qualification.

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

Comment 1

[[e-mail]]

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.

Comment 2

[ISHIKAWA, Chiaki]

Attachment: A patch (take 2) : Use the full namescope qualification.

(In reply to [e-mail] 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

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8409448 [details] [diff] [review]
A patch (take 2) : Use the full namescope qualification.

Thanks!

Comment 4

[ISHIKAWA, Chiaki]

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

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/57da6f0d047e

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/57da6f0d047e