New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 725726 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Fuchsia
Pri: 3
Type: Bug

Blocking:
issue 706592



Sign in to add a comment

Fuchsia/Magenta doesn't implement a thread naming API

Project Member Reported by scottmg@chromium.org, May 24 2017

Issue description

Chrome would like to implement

void PlatformThread::SetName(const std::string& name);

- pthread.h doesn't include pthread_setname_np (a gnu extension)
- threads.h has thrd_create_with_name() but it seems to be #ifdef'd out (or maybe we should be defining _ALL_SOURCE?). And that'd only be
- I didn't find a relevant syscall to set thread name.
 
Blocking: 706592
Ref upstream as MG-791.
We can define pthread_setname_np easily enough. It'll use the same guts that thrd_create_with_name does.

thrd_create_with_name is gated on _ALL_SOURCE, which I think we've put other things behind. My recollection is that I leaned a bit towards defining our own _MX_SOURCE or _FUCHSIA_SOURCE but was convinced by mcgrathr that we should just use _ALL_SOURCE. I think it makes very little difference to the Magenta or Fuchsia build itself what we pick, but we will guard nonstandard extensions behind _something_. So if you experience more or less pain based on the details of how we guard nonportable extensions, we can adjust.

Names on kernel objects (for some types that support it, which does include processes) are manipulated by mx_object_{get,set}_property: https://fuchsia.googlesource.com/magenta/+/HEAD/docs/syscalls/object_get_property.md#MX_PROP_NAME and in fact the c11 thread stuffs plumbs through to that.

This object property is what is used in our logging, the output of ps, etc. etc.
After a bit of typing and thought:

pthread_setname_np is easy, and I should do it since we have the C11 version. Presumably the Linux/OpenBSD-ish variant, which takes a pthread_t argument, rather than the OS X-ish one that only operates on the calling thread, as it's a bit more general.

I'd still suggest you cut out the middleman and operate on the mx_handle_t.
Status: Assigned (was: ExternalDependency)
Thanks! I'll use mx_object_set_property().
Project Member

Comment 6 by bugdroid1@chromium.org, May 24 2017

Comment 7 by w...@chromium.org, Jul 15 2017

Components: Internals>PlatformIntegration

Comment 8 by w...@chromium.org, Oct 14 2017

Owner: w...@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6124dadce4bd8c918cef7f7f7e5642faea83dac4

commit 6124dadce4bd8c918cef7f7f7e5642faea83dac4
Author: Wez <wez@chromium.org>
Date: Thu Oct 19 22:23:20 2017

Set the platform thread kernel object name in PlatformThread::SetName().

In addition to registering the name with the global ThreadIdManager, the
platform thread object's name is set with zx_object_set_property(), so
that it will be reflected in logging from e.g. the platform's crash
handler.

Bug:  725726 
Change-Id: Id47efa77857b35c2317b5eed05aa2e630a044113
Reviewed-on: https://chromium-review.googlesource.com/727699
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510238}
[modify] https://crrev.com/6124dadce4bd8c918cef7f7f7e5642faea83dac4/base/threading/platform_thread_fuchsia.cc
[modify] https://crrev.com/6124dadce4bd8c918cef7f7f7e5642faea83dac4/base/threading/platform_thread_unittest.cc

Comment 10 by w...@chromium.org, Oct 26 2017

Status: Fixed (was: Started)

Sign in to add a comment