In bug 1741131 we learnt that in some circumstances we end up seeing a parent process' annotation on a child process' crash report.

We assume that this happens only if an annotation was present on the parent but not on the child process and the crash report is submitted by the parent process on behalf of the killed child process.

It is also unclear (to the reporter) what happens in the inverse scenario, that is if child process' annotations will be reported correctly if they are not present on the parent process in the moment of submitting, too.

In bug 1741131 and for that specific annotation we can consider this almost to be a feature, but the level of confusion that this creates justifies some further investigation of options here. One option would be to add both (with a prefix). Another option would be to have some kind of opt-in to the behavior we currently see and exploit for XPCOMSpinEventLoopStack. Otherwise the child process' one should always prevail (even if it is missing in the parent).

See bug 1741131 comment 21.

This is an interesting use-case I never thought about before... and I feel silly that I haven't. Crash annotations in child processes work this way:

1. First we take all the annotations that have been set in the child process, these take precedence over everything else
2. If an annotation was not set in the child process, but is set in the main process then we pick the one in the main process (that's how most shared annotations work w/o having to duplicate the logic that fills them up in the child process)
3. We go over the annotations that have been set by the main process via CrashReporterHost::AddAnnotation() for the child process that crashes. These take precedence over whatever came out of step 2
4. Last but not least we manually add two more child-specific annotations before generating the crash report, see here

This is complicated and convoluted at the same time, we probably need a better system that covers at least these two types of annotations:

• Per process annotations: these should appear only in the process that set them, so for example if we set them in the main process they should not appear in a child process crash
• Shared annotations: these should be the same for every process (e.g. ProductName, Version, ...)

This could be addressed by adding an extra field to CrashAnnotations.yaml that specifies if an annotation is per-process or shared, let's call it scope, e.g.:

# ...

Version:
  description: >
    Product version.
  type: string
  scope: shared # This should appear in every process' crash report

# ...

XPCOMSpinEventLoopStack:
  description: >
    If we crash while some code is spinning manually the event loop on
    the main thread, we will see the stack of nested annotations here.
    If the crashing process was killed (e.g. due to an IPC error), this
    annotation may be out-of-date with the captured stack in the minidump,
    see bug 1741131 for details.
  type: string
  scope: process # This should appear only in the crash report associated with the process that set it

WDYT? Would this be sufficient to solve this issue?

(In reply to Gabriele Svelto [:gsvelto] from comment #2)

This is an interesting use-case I never thought about before... and I feel silly that I haven't.

No reason for feeling silly, it is certainly an edge use case if it came up only now.

Crash annotations in child processes work this way:

1. First we take all the annotations that have been set in the child process, these take precedence over everything else
2. If an annotation was not set in the child process, but is set in the main process then we pick the one in the main process (that's how most shared annotations work w/o having to duplicate the logic that fills them up in the child process)
3. We go over the annotations that have been set by the main process via CrashReporterHost::AddAnnotation() for the child process that crashes. These take precedence over whatever came out of step 2

Out of curiosity: Do you have an example on how this looks like in code? I do not see any child process selecting parameter in CrashReporterHost::AddAnnotation() ?

4. Last but not least we manually add two more child-specific annotations before generating the crash report, see here

This is complicated and convoluted at the same time, we probably need a better system that covers at least these two types of annotations:

• Per process annotations: these should appear only in the process that set them, so for example if we set them in the main process they should not appear in a child process crash
• Shared annotations: these should be the same for every process (e.g. ProductName, Version, ...)
  WDYT? Would this be sufficient to solve this issue?

I think it would, though in the specific case I would need to make XPCOMSpinEventLoopStack shared in order to keep the behavior I see currently (which is still not very intuitive, but ok). I assume it would be technically more difficult to have a scope like involved-processes that shows us both values, if they exist? This is kind of what I half-emulated with patch D131719 adding the process type, though it obviously can show up only for one process now. I think it would be ok to just join the two strings with some delimiter like [parent: xxxx][child: yyyy] to avoid hassle in the UI.

(In reply to Jens Stutte [:jstutte] from comment #3)

Out of curiosity: Do you have an example on how this looks like in code? I do not see any child process selecting parameter in CrashReporterHost::AddAnnotation() ?

Each CrashReporterHost object represents a child process, so the annotation is added to the crash report corresponding to that process.

I think it would, though in the specific case I would need to make XPCOMSpinEventLoopStack shared in order to keep the behavior I see currently (which is still not very intuitive, but ok). I assume it would be technically more difficult to have a scope like involved-processes that shows us both values, if they exist? This is kind of what I half-emulated with patch D131719 adding the process type, though it obviously can show up only for one process now. I think it would be ok to just join the two strings with some delimiter like [parent: xxxx][child: yyyy] to avoid hassle in the UI.

That would require significant surgery, not only to the code in mozilla-central but also in Socorro. We don't really have a good solution for annotations containing data coming from two different processes. We'd need to rethink how we gather them. It's good that you pointed out this particular use-case now before we start rewriting the client-side crash reporting machinery, I'll try to take it into account.