1714465 - Teach breakpad to populate ThreadNamesStream on Linux

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, task)

Description

[Aria Beingessner [:Gankra]]

I have implemented the parser for this in rust-minidump, and it picks up the thread names we set on Windows. Just need to grab the values on other platforms and build the stream.

It's a standard list stream containing entries with the layout:

{
  thread_id: u32,
  thread_name_rva: u64, (yes, it's an actual RVA64!)
}

With this done we can drop the need for the "extra" json file that we stuff thread names in.

Comment 1

[Aria Beingessner [:Gankra]]

Hmm, so outside of windows we aren't really hanging these thread names of OS APIs we can query by thread id. Instead the names are given directly to the crashreporter and stored as "thread annotations" that you can get with GetFlatThreadAnnotation.

So I guess I need to setup some way to get the data from GetFlatThreadAnnotation into breakpad? Hmm...

Comment 2

[Gabriele Svelto [:gsvelto]]

There's a few different ways of doing this on Linux (and on macOS too but I'll leave that for bug 1658831) without relying on GetFlatThreadAnnotation(). Ideally I'd like to move away from it because its code is complicated and brittle. We wrote it at a time where we didn't really have better options. Using OS facilities also has the added benefit that other debugging/monitoring tools will pick up the names, something that won't happen with the old system.

One way to do this under Linux is to use pthread_setname_np() to set the thread name. Note that NSPR's PR_CreateThread() already does it via NS_SetCurrentThreadName() which I believe we call on every new thread. A limitation of pthread_setname_np() is that by default it's capped to 16 characters including the null terminator. This shouldn't be a problem in practice as it seems that most of the API clients already take that into account (see how we have threads called ImageBridgeChld which seems to have been shortened just so that it fits in 15 characters).

Internally pthread_setname_np() uses prctl(PR_SET_NAME, ...) to talk directly to the kernel which poses a second issue, that is the names aren't kept in userspace. You can use pthread_getname_np() or prctl(PR_GET_NAME, ...) to retrieve them but only from the calling thread. Fortunately all thread information is surfaced via the /proc/<pid>/task/<tid> interface which we already iterate over here.

Long story short. If I'm not mistaken about our internal use of pthread_setname_np() this should be just a matter of augmenting the minidump writer to iterate over the thread names and write them into the appropriate stream.

Comment 3

[Gabriele Svelto [:gsvelto]]

I filed a corresponding bug in the minidump_writer_linux crate.

Comment 4

[msirringhaus]

Attachment: Bug 1714465 - Add ThreadNamesStream support to minidump_writer_linux. r=gsvelto

Comment 5

[Aria Beingessner [:Gankra]]

Oops forgot to reassign this to Gabriele, who seems to be on top of this.

Comment 6

[Gabriele Svelto [:gsvelto]]

Actually :msirringhaus already implemented this for Linux while I'm doing the macOS part in bug 1658831. I'm re-assigning this to him and I'll land his patch which somehow I forgot to land. Sorry for the delay.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/34bb4b92200c
Add ThreadNamesStream support to minidump_writer_linux. r=gsvelto

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/34bb4b92200c