classification
Title: thread.get_ident() should return unsigned value
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: amaury.forgeotdarc, gregory.p.smith, haypo, pitrou, rnk, sargo, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2009-07-21 14:25 by sargo, last changed 2017-03-24 17:22 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
thread_id_unsigned.patch serhiy.storchaka, 2013-02-06 14:38 review
thread_id_unsigned_3.patch serhiy.storchaka, 2015-02-18 07:27 review
thread_id_unsigned_4.patch serhiy.storchaka, 2015-02-18 12:34 review
Pull Requests
URL Status Linked Edit
PR 781 merged serhiy.storchaka, 2017-03-23 11:07
Messages (12)
msg90761 - (view) Author: Wojciech Lichota (sargo) Date: 2009-07-21 14:25
In glibc library (on linux) pthread_t is defined as:

typedef unsigned long int pthread_t;

But python thread module interprets this value as signed long. 

Reproduce:
>>> import thread
>>> thread.get_ident()
In some cases it returns negative value.
Checked in python 2.4, 2.5, 2.6

Proposal:
In my opinion code that cast value returned by pthread_self() should be
changed (see: Python/thread_pthread.h).

Other possibility is to change only returned value by get_ident
function. In this case it should use PyLong_FromUnsignedLong (see:
Modules/threadmodule.c).

Background:
logging module uses 'thread.get_ident()' to save thread_id in logs. If
the same application uses some C library that also writes in log file
some info with thread_id, in some situations this numbers are diffrent.
This complicate logs analyze.
msg112303 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-08-01 10:01
Can a C guru comment on this please.
msg125442 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-05 18:03
Well, the issue is that signedness differs depending on the platform. Under Windows, thread ids are signed (DWORD). Satisfying all cases would complicate things quite a bit.
msg125443 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-01-05 18:07
no, DWORD is a 32-bit unsigned integer
http://msdn.microsoft.com/en-us/library/aa383751(VS.85).aspx
msg125445 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-05 18:17
> no, DWORD is a 32-bit unsigned integer
> http://msdn.microsoft.com/en-us/library/aa383751(VS.85).aspx

Oops, my bad.
msg181526 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-02-06 14:38
Here is a patch, which made all thread id to be unsigned.
msg181548 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-02-06 18:07
You could add a test for it.
msg236157 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-02-17 23:59
On Linux, the following C program tells me that pthread_t is unsigned.
---
#include <pthread.h>
#include <stdio.h>

#define TYPE_IS_SIGNED(TYPE) ((TYPE)-1 < (TYPE)0)

int main()
{
    printf("signed? %i\n", TYPE_IS_SIGNED(pthread_t));
    return 0;
}
---

So it's fair to modify threading.get_ident() to return an unsigned number.

But I disagree to change stable Python versions, it may break applications.

Oh, I wrote write_thread_id() in Python/traceback.c and this function already casts the thread identifier to an unsigned number ;-)
msg236165 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-18 07:27
Here is updated patch. Added few tests.
msg236166 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-02-18 08:31
> Here is updated patch. Added few tests.

Cool. I sent a review.
msg236169 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-02-18 12:34
Thank you Victor for your review. Here is updated patch.
msg290096 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-03-24 16:32
New changeset aefa7ebf0ff0f73feee7ab24f4cdcb2014d83ee5 by Victor Stinner (Serhiy Storchaka) in branch 'master':
bpo-6532: Make the thread id an unsigned integer. (#781)
https://github.com/python/cpython/commit/aefa7ebf0ff0f73feee7ab24f4cdcb2014d83ee5
History
Date User Action Args
2017-03-24 17:22:55serhiy.storchakasetstatus: open -> closed
assignee: serhiy.storchaka
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.7, - Python 3.5
2017-03-24 16:32:00hayposetmessages: + msg290096
2017-03-23 11:07:48serhiy.storchakasetpull_requests: + pull_request685
2015-02-18 12:34:31serhiy.storchakasetfiles: + thread_id_unsigned_4.patch

messages: + msg236169
2015-02-18 08:31:49hayposetmessages: + msg236166
2015-02-18 07:27:52serhiy.storchakasetfiles: + thread_id_unsigned_3.patch

messages: + msg236165
2015-02-17 23:59:19hayposetnosy: + haypo

messages: + msg236157
versions: + Python 3.5, - Python 2.7, Python 3.2, Python 3.3, Python 3.4
2014-02-03 18:40:59BreamoreBoysetnosy: - BreamoreBoy
2013-02-06 18:07:53pitrousetmessages: + msg181548
2013-02-06 14:38:16serhiy.storchakasetfiles: + thread_id_unsigned.patch
keywords: + patch
messages: + msg181526

stage: needs patch -> patch review
2013-02-06 10:09:21serhiy.storchakasetnosy: + serhiy.storchaka

versions: + Python 3.3, Python 3.4, - Python 3.1
2011-01-05 18:17:00pitrousetnosy: gregory.p.smith, amaury.forgeotdarc, pitrou, rnk, sargo, BreamoreBoy
messages: + msg125445
2011-01-05 18:07:47amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg125443
2011-01-05 18:03:37pitrousetnosy: + gregory.p.smith, rnk, pitrou
messages: + msg125442
2010-08-01 10:01:59BreamoreBoysettype: behavior
components: + Extension Modules, - Library (Lib)
versions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.6, Python 2.5, Python 2.4
nosy: + BreamoreBoy

messages: + msg112303
stage: needs patch
2009-07-21 14:25:21sargocreate