thread.get_ident() should return unsigned value

[sargo]

BPO	6532
Nosy	@gpshead, @amauryfa, @pitrou, @vstinner, @rnk, @serhiy-storchaka
PRs	bpo-6532: Make the thread id an unsigned integer. #781
Files	thread_id_unsigned.patch thread_id_unsigned_3.patch thread_id_unsigned_4.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-03-24.17:22:55.071>
created_at = <Date 2009-07-21.14:25:21.052>
labels = ['extension-modules', 'type-bug', '3.7']
title = 'thread.get_ident() should return unsigned value'
updated_at = <Date 2017-03-24.17:22:55.070>
user = 'https://bugs.python.org/sargo'

bugs.python.org fields:

activity = <Date 2017-03-24.17:22:55.070>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-03-24.17:22:55.071>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2009-07-21.14:25:21.052>
creator = 'sargo'
dependencies = []
files = ['28976', '38167', '38168']
hgrepos = []
issue_num = 6532
keywords = ['patch']
message_count = 12.0
messages = ['90761', '112303', '125442', '125443', '125445', '181526', '181548', '236157', '236165', '236166', '236169', '290096']
nosy_count = 7.0
nosy_names = ['gregory.p.smith', 'amaury.forgeotdarc', 'pitrou', 'vstinner', 'rnk', 'sargo', 'serhiy.storchaka']
pr_nums = ['781']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue6532'
versions = ['Python 3.7']

[sargo]

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.

[BreamoreBoy]

Can a C guru comment on this please.

[pitrou]

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.

[amauryfa]

no, DWORD is a 32-bit unsigned integer
http://msdn.microsoft.com/en-us/library/aa383751(VS.85).aspx

[pitrou]

no, DWORD is a 32-bit unsigned integer
http://msdn.microsoft.com/en-us/library/aa383751(VS.85).aspx

Oops, my bad.

[serhiy-storchaka]

Here is a patch, which made all thread id to be unsigned.

[pitrou]

You could add a test for it.

[vstinner]

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 ;-)

[serhiy-storchaka]

Here is updated patch. Added few tests.

[vstinner]

Here is updated patch. Added few tests.

Cool. I sent a review.

[serhiy-storchaka]

Thank you Victor for your review. Here is updated patch.

[vstinner]

New changeset aefa7eb by Victor Stinner (Serhiy Storchaka) in branch 'master':
bpo-6532: Make the thread id an unsigned integer. (#781)
aefa7eb