1181595 - crash in mozilla::dom::MessagePort::RemoveDocFromBFCache()

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Tracy Walker [:tracy]]

This bug was filed from the Socorro interface and is 
report bp-7031611a-073b-40ef-913d-41e0f2150703.
=============================================================

This first appeared on Nightly builds of 2015061803. It ramped up in volume once merged to Aurora with builds from 2015063003

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

null pointer + offset crash?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

looks like so.

Comment 3

[Andrea Marchesini [:baku]]

Attachment: mc2.patch

Comment 4

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce51b85a39bc

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ce51b85a39bc

Comment 6

[(Away)]

Can you please uplift to aurora?

Comment 7

[Andrea Marchesini [:baku]]

Comment on attachment 8631115 [details] [diff] [review]
mc2.patch

Approval Request Comment
[Feature/regressing bug #]: MessagePort/MessageChannel
[User impact if declined]: a crash
[Describe test coverage new/current, TreeHerder]: none. it's very racy.
[Risks and why]: none. the patch replaces a MOZ_ASSERT() with a if(..) return;
[String/UUID change made/needed]: none.

Comment 8

[Kate Glazko]

Comment on attachment 8631115 [details] [diff] [review]
mc2.patch

Minimal risk, the patch replaces a MOZ_ASSERT() with a if(..) return; and is not causing issues on m-c.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8cfc3238ee5e

Comment 10

[Robert Kaiser]

The signature still shows up in 41.0b1: https://crash-stats.mozilla.com/signature/?product=Firefox&version=41.0b1&signature=mozilla%3A%3Adom%3A%3AMessagePort%3A%3ARemoveDocFromBFCache%28%29&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&page=1

Andrea, do we need to reopen this?

Comment 11

[(Away)]

I would say yes. (Or a new bug; I don't care which)

In those crash reports, it is not |window| that is null. It is |this| that is null, which means that |mPort| is null here:

bool
MessagePortChild::RecvReceiveData(nsTArray<MessagePortMessage>&& aMessages)
{
  MOZ_ASSERT(mPort);
  mPort->MessagesReceived(aMessages);
  return true;
}

Comment 12

[Andrea Marchesini [:baku]]

What I suspect is happening here is:

1. port1 is sending data
2. port2.close() -> we set mPort to null in the actor
3. port2 receives data before that the close() operation is managed by the MessagePortService.

Comment 13

[Andrea Marchesini [:baku]]

Attachment: crash.patch

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8648058 [details] [diff] [review]
crash.patch

I would have opened a new bug for this patch. Easier to track what has been fixed and where.

Comment 15

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6411f2fcd3ec

Comment 16

[Andrea Marchesini [:baku]]

Comment on attachment 8648058 [details] [diff] [review]
crash.patch

Approval Request Comment
[Feature/regressing bug #]: MessagePort/Channel
[User impact if declined]: a crash can happen
[Describe test coverage new/current, TreeHerder]: race condition
[Risks and why]: no risk. We just add a if() check.
[String/UUID change made/needed]: none

Comment 17

[(Away)]

Only aurora? This crash signature is seen on beta as well. (v41 has become beta in the time since comment 6)

Comment 18

[Andrea Marchesini [:baku]]

Comment on attachment 8648058 [details] [diff] [review]
crash.patch

Approval Request Comment
[Feature/regressing bug #]: MessagePort/Channel
[User impact if declined]: a crash can happen
[Describe test coverage new/current, TreeHerder]: race condition hard to reproduce.
[Risks and why]: none 
[String/UUID change made/needed]: none

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8648058 [details] [diff] [review]
crash.patch

Crash fix, let's uplift to Aurora and Beta.

Comment 20

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

https://hg.mozilla.org/mozilla-central/rev/6411f2fcd3ec

Comment 23

[Justin Wood (:Callek)]

Landed on beta https://hg.mozilla.org/releases/mozilla-beta/rev/e393da8ff81b

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d43981e7c3ab