1035144 - Add AsyncShutdownTimeout to the crash signature

Status: RESOLVED FIXED

(Socorro :: General, task)

Description

[David Teller [:Yoric] - still alive but not very active]

At the moment, all the AsyncShutdownTimeout crashes have the same signature, regardless of their cause which makes the signature useless/counter-productive (e.g. bug 944873 was about a specific crash and has completely been hijacked by other crashes with the same signature).

For these crashes, neither the native stack nor the native signature make sense, but only the contents of metadata `AsyncShutdownTimeout`, which looks as follows

{
  phase: string,
  conditions: array of [
    name: string, // This is the most important piece of information
    state: either arbitrary JSON or the string "(none)",
    // Since FF 33
    lineNumber: number,
    filename: string,
    // In FF 34+, we will probably add
    stack: string
  ]
}

I believe that it would be more productive if we had the following signature:

AsyncShutdownTimeout | name [, name, name ...]

Comment 1

[Robert Kaiser]

Benjamin, what do you think of this change?

Comment 2

[Benjamin Smedberg]

I support this change, with the minor nit that it should be |-separated

AsyncShutdownTimeout | name [| name ...]

If the AsyncShutdownTimeout annotation is not valid JSON or has a 0-length conditions field (or other unexpected errors), the signature should be "AsyncShutdownTimeout | UNKNOWN"

https://github.com/mozilla/socorro/commit/6881b36682abf0041db8841cc6743932f6bb1fdf is an recent example of how to make these kinds of signature modifications. This one should be quite simple. Lars do you have any webdev/socorro volunteers you could mentor for this?

Comment 3

[David Teller [:Yoric] - still alive but not very active]

As a side-note, once bug 1034726 has landed (today?), we might wish to expose the JavaScript stacks alongside with (or instead of) the C++ stacks.

Comment 4

[Benjamin Smedberg]

Let's not make that part of this bug.

Comment 5

[K Lars Lohn [:lars] [:klohn]]

Benjamin, I unfortunately do not have a volunteer/apprentice/protege/slave eager and willing to write such code.  You are correct that this is likely to be quite simple - in your reference to the git hub commit, this new transform rule will be patterned after  he OOM signature rules found on lines 138-180 in the commit that you reference.

Comment 6

[Bogdan L.]

Hi,
I would like to work on this :)

Comment 7

[Anusha]

Hi,
I would like to work on this bug,
Thanks.

Comment 8

[Benjamin Smedberg]

Yoric, this came up again recently and I was looking through some recent crash reports. See https://crash-stats.mozilla.com/report/index/9d500c33-c15d-458c-b471-bd1ea2160823 for an example of a crash that has no active conditions in the "Places Clients shutdown" block. Does that make any sense/do you have suggestions for figuring that out?

Comment 9

[Benjamin Smedberg]

https://github.com/mozilla/socorro/pull/3443

This is an untested patch without unit tests. Can somebody from the socorro team take this across the finish line, since I don't have a dev environment?

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

I would love to have this fix in Socorro, so that we can better understand reports and apparent spikes in crashes like we see right now in bug 1255745.

Comment 11

[Peter Bengtsson [:peterbe]]

(In reply to Benjamin Smedberg [:bsmedberg] from comment #9)
> https://github.com/mozilla/socorro/pull/3443
> 
> This is an untested patch without unit tests. Can somebody from the socorro
> team take this across the finish line, since I don't have a dev environment?

I'll write a unit test towards your PR but perhaps we can wait till Monday (next week) for Adrian to do a final review and merge.

Comment 12

[Peter Bengtsson [:peterbe]]

https://github.com/bsmedberg/socorro/pull/1 for unit tests on bsmedbergs PR.

Comment 13

[David Teller [:Yoric] - still alive but not very active]

(In reply to Benjamin Smedberg [:bsmedberg] from comment #8)
> Yoric, this came up again recently and I was looking through some recent
> crash reports. See
> https://crash-stats.mozilla.com/report/index/9d500c33-c15d-458c-b471-
> bd1ea2160823 for an example of a crash that has no active conditions in the
> "Places Clients shutdown" block. Does that make any sense/do you have
> suggestions for figuring that out?

That's a bit surprising. It should mean that the problem is in "Places Clients shutdown" itself, rather than any of its blockers, but I can't see how this can happen in this barrier.

So, in all likelihood, this indicates a bug in the state-reporting method of this barrier: `PlacesShutdownBlocker::GetState`. We should check this method and this will probably help us find out what causes the timeout.

Comment 14

[[github robot]]

Commits pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/6f7f79ef5e57c4fbf6b0e67e10d4fc0468684219
Bug 1035144 - Modify signatures for AsyncShutdownTimeout to represent the shutdown conditions instead of the stack.

https://github.com/mozilla/socorro/commit/9498594e122431834deefbde3d9767d6b6127fb3
tests for bug 1035144

https://github.com/mozilla/socorro/commit/6a82c599769cb4d286a626b0758b599a669f55dc
Merge pull request #1 from peterbe/tests-for-bug-1035144

tests for bug 1035144

https://github.com/mozilla/socorro/commit/3aa8728b1f51c498cb39009ffb884c8b413f6a38
Merge pull request #2 from adngdb/bug1035144

Removed inheritence from SignatureGenerationRule.

https://github.com/mozilla/socorro/commit/344979da66b185fc61ba660e9848591bf6a1ef14
Bug 1035144 - Modify signatures for AsyncShutdownTimeout to represent the shutdown conditions instead of the stack. (#3443)

* Bug 1035144 - Modify signatures for AsyncShutdownTimeout to represent the shutdown conditions instead of the stack.

* tests for bug 1035144

* Removed inheritence from SignatureGenerationRule.

* review

* fix test