bpo-31596: Add an interface for pthread_getcpuclockid(3) #3756

pdox · 2017-09-26T03:15:25Z

Modeled on the interface for signal.pthread_kill

https://bugs.python.org/issue31596

the-knights-who-say-ni · 2017-09-26T03:15:27Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

pdox · 2017-09-27T05:25:38Z

FYI, I signed CLA. Not sure why the label hasn't cleared.

benjaminp · 2017-09-27T06:07:34Z

Thanks for the PR. Unfortunately, I think signal.pthread_kill is a rather broken API. At least in nptl, pthread_t is just a pointer, so pthread_kill may be used to dereference arbitrary memory.

>>> signal.pthread_kill(42, signal.SIGKILL)
Segmentation fault

It's a design flaw of threading that it exposes thread identifiers as integers at all. Even in C, pthreads specifies pthread_t to be opaque to the application. So, for this particular issue, I see two ways forward: 1) add a method to threading.Thread that returns the clock id 2) properly expose an opaque object for thread identifiers and have time.pthread_getcpuclockid take such an object.

pdox · 2017-09-29T00:42:00Z

Ah, we meet again. I should have known you wouldn't go easy on me.

I've decided to give option 2 a try. I have created https://bugs.python.org/issue31622 to track it, and will be sending a pull request shortly.

benjaminp

It seems others as not as concerned as me about identifiers, so I suppose we can move forward here. lgtm except for a few nits.

benjaminp · 2017-10-02T06:05:30Z

Doc/library/time.rst

+
+   Return the *clk_id* of the thread-specific CPU-time clock for the specified *thread_id*.
+
+   Use :func:`threading.get_ident()` or the :attr:`~threading.Thread.ident`


No parens after the function name.

benjaminp · 2017-10-02T06:05:30Z

Doc/library/time.rst

+   for *thread_id*.
+
+   .. warning::
+      Passing an invalid or expired thread_id may result in


*thread_id*

benjaminp · 2017-10-02T06:05:30Z

Modules/timemodule.c

+        PyErr_SetFromErrno(PyExc_OSError);
+        return NULL;
+    }
+    return PyLong_FromLong((long)clk_id);


This cast is superfluous. If clockid_t ends up being something large, it's probably good to get the compiler warning for the implicit cast.

pdox · 2017-10-05T01:03:53Z

[ping]. did I address all your concerns?

Add an interface for pthread_getcpuclockid

4066015

bedevere-bot added the awaiting review label Sep 26, 2017

the-knights-who-say-ni added the CLA not signed label Sep 26, 2017

Add NEWS entry

a102642

pdox changed the title ~~Add an interface for pthread_getcpuclockid(3)~~ bpo-31596: Add an interface for pthread_getcpuclockid(3) Sep 26, 2017

benjaminp removed the CLA not signed label Sep 27, 2017

the-knights-who-say-ni added the CLA signed label Sep 27, 2017

benjaminp reviewed Oct 2, 2017

View changes

address nits

6fe4f90

benjaminp merged commit e14679c into python:master Oct 5, 2017

bedevere-bot removed the awaiting review label Oct 5, 2017

bpo-31596: Add an interface for pthread_getcpuclockid(3) #3756

bpo-31596: Add an interface for pthread_getcpuclockid(3) #3756

pdox commented Sep 26, 2017 •

edited by bedevere-bot

the-knights-who-say-ni commented Sep 26, 2017

pdox commented Sep 27, 2017

benjaminp commented Sep 27, 2017

pdox commented Sep 29, 2017

benjaminp left a comment

benjaminp Oct 2, 2017

benjaminp Oct 2, 2017

benjaminp Oct 2, 2017

pdox commented Oct 5, 2017


		Return the clk_id of the thread-specific CPU-time clock for the specified thread_id.

		Use :func:`threading.get_ident()` or the :attr:`~threading.Thread.ident`

bpo-31596: Add an interface for pthread_getcpuclockid(3) #3756

bpo-31596: Add an interface for pthread_getcpuclockid(3) #3756

Conversation

pdox commented Sep 26, 2017 • edited by bedevere-bot

the-knights-who-say-ni commented Sep 26, 2017

pdox commented Sep 27, 2017

benjaminp commented Sep 27, 2017

pdox commented Sep 29, 2017

benjaminp left a comment

benjaminp Oct 2, 2017

Choose a reason for hiding this comment

benjaminp Oct 2, 2017

Choose a reason for hiding this comment

benjaminp Oct 2, 2017

Choose a reason for hiding this comment

pdox commented Oct 5, 2017

pdox commented Sep 26, 2017 •

edited by bedevere-bot