Closed Bug 750498 Opened 9 years ago Closed 9 years ago

Support IPC Thread::SetName on Linux

Categories

(Core :: IPC, enhancement)

All
Linux
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: nical, Assigned: nical)

References

()

Details

Attachments

(1 file, 1 obsolete file)

Similar to bug 734685 Which implemented the feature on Mac, for desktop Linux this time.
Attachment #619697 - Flags: review?(jones.chris.g)
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.
Attachment #619697 - Flags: review?(jones.chris.g) → review+
(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.
  prctl(PR_SET_NAME, reinterpret_cast<uintptr_t>(name), 0, 0, 0);

ought to work on both.
I tried this on ICS and the threads doesn't get renamed :(.
Your changes and gdb's 'info threads'
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.
Attachment #619697 - Attachment is obsolete: true
Attachment #620086 - Flags: review?(jones.chris.g)
Attachment #620086 - Flags: feedback?(bgirard)
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.
Attachment #620086 - Flags: feedback?(bgirard) → feedback+
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.
Attachment #620086 - Flags: review?(jones.chris.g) → review+
Attachment #620086 - Flags: checkin?(bgirard)
Jim, any idea why setting the thread name wouldn't show up in GDB?
Attachment #620086 - Flags: checkin?(bgirard) → checkin+
https://hg.mozilla.org/mozilla-central/rev/7cd83841136c
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Depends on: 752067
(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!
(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
You need to log in before you can comment on or make changes to this bug.