1274404 - Annotate crashes for large messages in ProcessLink::SendMessage()

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Andrew McCreight [:mccr8]]

The crash from trying to send a large IPC message (bug 1271102) is quite common. We should annotate these crashes with the name of the message, and possibly the message size, to better understand where these very large messages are coming from. The normal annotate crash report method can't be used, because that does IPC, but maybe we could have a mechanism like AnnotateMozCrashReason() to annotate the crash report.

Comment 1

[Eric Rahm [:erahm]]

See bug 1271102, comment 4. We can't do annotations off the main thread in the content process.

Comment 2

[Andrew McCreight [:mccr8]]

The mechanism used by AnnotateMozCrashReason can be run off the main thread AFAICT. It just sets a global, then some later function comes along and reads it out.

Comment 3

[Eric Rahm [:erahm]]

(In reply to Andrew McCreight [:mccr8] from comment #2)
> The mechanism used by AnnotateMozCrashReason can be run off the main thread
> AFAICT. It just sets a global, then some later function comes along and
> reads it out.

Okay, I can do this.

Comment 4

[Andrew McCreight [:mccr8]]

Thanks, Eric! Also, I think I'm going to go ahead and implement bug 1272423, which should reduce the frequency of these crashes.

Comment 5

[Eric Rahm [:erahm]]

As it turns out this was fixed recently, we can annotate off main thread now.

Comment 6

[Eric Rahm [:erahm]]

Attachment: Annotate IPC message name and size when crashing due to message size

Comment 7

[Andrew McCreight [:mccr8]]

Comment on attachment 8754611 [details] [diff] [review]
Annotate IPC message name and size when crashing due to message size

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

Thanks.

::: ipc/glue/MessageLink.cpp
@@ +157,5 @@
>  
>  void
>  ProcessLink::SendMessage(Message *msg)
>  {
> +    if (msg->size() > IPC::Channel::kMaximumMessageSize) {

nit: >=

Comment 8

[Eric Rahm [:erahm]]

(In reply to Andrew McCreight [:mccr8] from comment #7)
> Comment on attachment 8754611 [details] [diff] [review]
> Annotate IPC message name and size when crashing due to message size
> 
> Review of attachment 8754611 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks.
> 
> ::: ipc/glue/MessageLink.cpp
> @@ +157,5 @@
> >  
> >  void
> >  ProcessLink::SendMessage(Message *msg)
> >  {
> > +    if (msg->size() > IPC::Channel::kMaximumMessageSize) {
> 
> nit: >=

I think i had it wrong originally, logically we only care if it's greater than the max size.

Comment 9

[Andrew McCreight [:mccr8]]

(In reply to Eric Rahm [:erahm] from comment #8)
> I think i had it wrong originally, logically we only care if it's greater
> than the max size.

Ah, of course! Sorry.

Comment 10

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2a2a4005e9356ccd2965bb47c1655d33193a7635
Bug 1274404 - Annotate IPC message name and size when crashing due to message size. r=mccr8

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for address sanitizer bustage on both opt and debug:

https://hg.mozilla.org/integration/mozilla-inbound/rev/64bcbab8fa95a965837dee39d297fbc6f2117fc4
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=2a2a4005e9356ccd2965bb47c1655d33193a7635

MessageLink.cpp:162:7: error: use of undeclared identifier 'CrashReporter'

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Probably should have both:
  #ifdef MOZ_CRASHREPORTER
  #endif
around the annotation code, and also:
  #ifdef MOZ_CRASHREPORTER
  #include "nsExceptionHandler.h"
  #endif

Comment 13

[Eric Rahm [:erahm]]

Attachment: Annotate IPC message name and size when crashing due to message size

Comment 14

[Eric Rahm [:erahm]]

Comment on attachment 8755535 [details] [diff] [review]
Annotate IPC message name and size when crashing due to message size

Updated per comment 12. Carrying forward r+

Comment 15

[Pulsebot]

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

Comment 16

[Carsten Book [:Tomcat]]

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

Comment 17

[Andrew McCreight [:mccr8]]

The telemetry is working (you have to be logged into crash-stats with minidump permissions in order to see these fields):
https://crash-stats.mozilla.com/report/index/4f46e160-9a9b-46e7-abad-66e122160526

IPCMessageName: PBrowser::Msg_InvokeDragSession
IPCMessageSize: 434290984

434MB! Fortunately, bug 1272018 and bug 1275398 should fix this situation.

Comment 18

[Andrew McCreight [:mccr8]]

Err, not telemetry, crash annotation.