1179685 - Assert/Crash running "postmessage-msgport-to-client.https.html" wpt service worker test in Nightly

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Noemí Freire (:noemi)]

Attachment: Nightly_Crash_wpt_sw_postmessage-msgport-to-client.https.rtf

A Nightly crash occurs when executing "postmessage-msgport-to-client.https.html" wpt test such as |./mach web-platform-tests _mozilla/service-workers/service-worker/postmessage-msgport-to-client.https.html| with 7/1 master build

The assertion failure shown is as follows:
""Assertion failure: aClosure, at /Users/noef/Documents/mozilla-central/dom/workers/WorkerPrivate.cpp:693""

Please find attached the crash report corresponding to this

Comment 1

[(no longer active)]

What is happening here is that we call mBuffer.read() here <https://dxr.allizom.org/mozilla-central/source/dom/workers/ServiceWorkerClient.cpp#130> passing nullptr as the closure argument (it is a default argument that we are passing in explicitly here) and in WorkerStructuredCloneCallbacks::ReadTransfer() <https://dxr.allizom.org/mozilla-central/source/dom/workers/WorkerPrivate.cpp#693> we MOZ_ASSERT(aClosure).  In non-debug builds, if we end up with a SCTAG_DOM_MAP_MESSAGEPORT tag, we will crash.

Catalin, is this expected?  It seems like passing a null closure to this function is a mistake.

Comment 2

[Cătălin Badea (:catalinb)]

Attachment: Fix messageport assert in wpt service worker test.

Comment 3

[Cătălin Badea (:catalinb)]

(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #1)
> What is happening here is that we call mBuffer.read() here
> <https://dxr.allizom.org/mozilla-central/source/dom/workers/
> ServiceWorkerClient.cpp#130> passing nullptr as the closure argument (it is
> a default argument that we are passing in explicitly here) and in
> WorkerStructuredCloneCallbacks::ReadTransfer()
> <https://dxr.allizom.org/mozilla-central/source/dom/workers/WorkerPrivate.
> cpp#693> we MOZ_ASSERT(aClosure).  In non-debug builds, if we end up with a
> SCTAG_DOM_MAP_MESSAGEPORT tag, we will crash.
> 
> Catalin, is this expected?  It seems like passing a null closure to this
> function is a mistake.

No, that's a bug. I've uploaded a patch that fixes the issue.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/35aee276e08f

Comment 5

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

WPT(4) is failing, so backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/6b3e9fe5e044


https://treeherder.mozilla.org/logviewer.html#?job_id=11436547&repo=mozilla-inbound

Comment 6

[Cătălin Badea (:catalinb)]

Attachment: Change service worker postmsg-msgport-to-client wpt test expectation to PASS.

Comment 7

[(no longer active)]

Comment on attachment 8631157 [details] [diff] [review]
Change service worker postmsg-msgport-to-client wpt test expectation to PASS.

Review of attachment 8631157 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, usually when the test expectation file indicates that it passes everywhere, you can remove the ini file completely.  So please do that if the test passes with that!

Comment 8

[Cătălin Badea (:catalinb)]

Attachment: Change service worker postmsg wpt test expectation to PASS.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bbf6fa4325a4
https://hg.mozilla.org/integration/mozilla-inbound/rev/34b09aa0ec63

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bbf6fa4325a4
https://hg.mozilla.org/mozilla-central/rev/34b09aa0ec63

Comment 11

[Noemí Freire (:noemi)]

Hi,

just checked on m-c Nightly Debug (72835344333f revision), and the test is successfully executed. Thanks!

*Harness status: OK
*Found 1 tests
*1 Pass