The default bug view has changed. See this FAQ.

Support IPC Thread::SetName on Linux

RESOLVED FIXED in mozilla15

Status

()

Core
IPC
--
enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nical, Assigned: nical)

Tracking

unspecified
mozilla15
All
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 619697 [details] [diff] [review]
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.
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+
(Assignee)

Comment 2

5 years ago
(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 :(.
How did you test?
Your changes and gdb's 'info threads'
(Assignee)

Comment 7

5 years ago
Created attachment 620086 [details] [diff] [review]
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.
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+
(Assignee)

Updated

5 years ago
Attachment #620086 - Flags: checkin?(bgirard)
Jim, any idea why setting the thread name wouldn't show up in GDB?
https://hg.mozilla.org/integration/mozilla-inbound/rev/7cd83841136c

Updated

5 years ago
Attachment #620086 - Flags: checkin?(bgirard) → checkin+
https://hg.mozilla.org/mozilla-central/rev/7cd83841136c
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15

Updated

5 years ago
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!
Thanks, filled http://sourceware.org/bugzilla/show_bug.cgi?id=14495
(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.