750498 - Support IPC Thread::SetName on Linux

Status: RESOLVED FIXED

(Core :: IPC, enhancement)

Description

[Nicolas Silva [:nical]]

Attachment: Applies upstream change to Thread::SetName for thread names to be displayable in gdb on Linux.

Similar to bug 734685 Which implemented the feature on Mac, for desktop Linux this time.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 619697 [details] [diff] [review]
Applies upstream change to Thread::SetName for thread names to be displayable in gdb on Linux.

bionic supports prctl(PR_SET_NAME), and so should maemo.  No need to exclude them here.  Leaving the original condition as |#ifndef OS_MACOSX| should be fine.

r=me with that change.

Comment 2

[Nicolas Silva [:nical]]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #1)
> bionic supports prctl(PR_SET_NAME), and so should maemo.  No need to exclude
> them here.  Leaving the original condition as |#ifndef OS_MACOSX| should be
> fine.

I tried this already and it breaks the build on android and b2g platforms because prctl apparently doesn't have the same signature (it takes a unsigned long int instead of a c string). 
I don't know about maemo because I don't think try servers include maemo build and I haven't found a wiki page explaining how to build for maemo. so I did not want to risk to break it. I'll ask romaxa about the maemo stuff.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

  prctl(PR_SET_NAME, reinterpret_cast<uintptr_t>(name), 0, 0, 0);

ought to work on both.

Comment 4

[Benoit Girard (:BenWa)]

I tried this on ICS and the threads doesn't get renamed :(.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

How did you test?

Comment 6

[Benoit Girard (:BenWa)]

Your changes and gdb's 'info threads'

Comment 7

[Nicolas Silva [:nical]]

Attachment: SetName calls prctl on all linux platforms including mobile.

This is the version with prctl(PR_SET_NAME, reinterpret_cast<uintptr_t>(name), 0, 0, 0); on all Linux platforms.

May not actually show the thread names in gdb for android (from BenWa's comment) but it doesn't break on try server so it's better than the previous attachment anyway.

Comment 8

[Benoit Girard (:BenWa)]

Comment on attachment 620086 [details] [diff] [review]
SetName calls prctl on all linux platforms including mobile.

It compiled and ran fine so its worth checking in. Maybe later we can dig into why it didn't work on android.

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I wouldn't be the least bit surprised if the crack-baby android gdb doesn't read thread names.  If attaching to a process that had already spawned threads worked reliably, we could check and see if gdb reads these names when it first looks for threads.

Would be worth poking at in jimdb.

Comment 10

[Benoit Girard (:BenWa)]

Jim, any idea why setting the thread name wouldn't show up in GDB?

Comment 11

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7cd83841136c

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7cd83841136c

Comment 13

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Benoit Girard (:BenWa) from comment #10)
> Jim, any idea why setting the thread name wouldn't show up in GDB?

Sorry for the late reply. I think the issue is that gdbserver does not support thread names. Even on Linux, I don't see thread names when I switch from gdb to gdbserver. Thanks!

Comment 14

[Benoit Girard (:BenWa)]

Thanks, filled http://sourceware.org/bugzilla/show_bug.cgi?id=14495

Comment 15

[Benoit Girard (:BenWa)]

(In reply to Benoit Girard (:BenWa) from comment #14)
> Thanks, filled http://sourceware.org/bugzilla/show_bug.cgi?id=14495

Got an update from this bug, this is in progress:
This is part of http://sourceware.org/gdb/wiki/LocalRemoteFeatureParity