Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-112535: Add test on _Py_ThreadId() #112709

Merged
merged 4 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Lib/test/support/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,8 @@ def check_cflags_pgo():
return any(option in cflags_nodist for option in pgo_options)


if sysconfig.get_config_var('Py_GIL_DISABLED'):
Py_GIL_DISABLED = bool(sysconfig.get_config_var('Py_GIL_DISABLED'))
if Py_GIL_DISABLED:
_header = 'PHBBInP'
else:
_header = 'nP'
Expand Down
55 changes: 55 additions & 0 deletions Lib/test/test_capi/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -2854,5 +2854,60 @@ def testfunc(n, m):
self.assertIn("_FOR_ITER_TIER_TWO", uops)


@unittest.skipUnless(support.Py_GIL_DISABLED, 'need Py_GIL_DISABLED')
class TestPyThreadId(unittest.TestCase):
def test_py_thread_id(self):
# gh-112535: Test _Py_ThreadId(): make sure that thread identifiers
# in a few threads are unique
py_thread_id = _testinternalcapi.py_thread_id
short_sleep = 0.010

class GetThreadId(threading.Thread):
def __init__(self):
super().__init__()
self.get_lock = threading.Lock()
self.get_lock.acquire()
self.started_lock = threading.Event()
self.py_tid = None

def run(self):
self.started_lock.set()
self.get_lock.acquire()
self.py_tid = py_thread_id()
time.sleep(short_sleep)
self.py_tid2 = py_thread_id()

nthread = 5
threads = [GetThreadId() for _ in range(nthread)]

# first make run sure that all threads are running
for thread in threads:
thread.start()
for thread in threads:
thread.started_lock.wait()

# call _Py_ThreadId() in the main thread
py_thread_ids = [py_thread_id()]

# now call _Py_ThreadId() in each thread
for thread in threads:
thread.get_lock.release()

# call _Py_ThreadId() in each thread and wait until threads complete
for thread in threads:
thread.join()
py_thread_ids.append(thread.py_tid)
# _PyThread_Id() should not change for a given thread.
# For example, it should remain the same after a short sleep.
self.assertEqual(thread.py_tid2, thread.py_tid)

# make sure that all _Py_ThreadId() are unique
for tid in py_thread_ids:
self.assertIsInstance(tid, int)
self.assertGreater(tid, 0)
self.assertEqual(len(set(py_thread_ids)), len(py_thread_ids),
py_thread_ids)


if __name__ == "__main__":
unittest.main()
3 changes: 1 addition & 2 deletions Lib/test/test_cppext/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# compatible with C++ and does not emit C++ compiler warnings.
import os.path
import shutil
import sys
import unittest
import subprocess
import sysconfig
Expand All @@ -15,7 +14,7 @@

# gh-110119: pip does not currently support 't' in the ABI flag use by
# --disable-gil builds. Once it does, we can remove this skip.
@unittest.skipIf(sysconfig.get_config_var('Py_GIL_DISABLED') == 1,
@unittest.skipIf(support.Py_GIL_DISABLED,
'test does not work with --disable-gil')
@support.requires_subprocess()
class TestCPPExt(unittest.TestCase):
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_importlib/test_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import os
import re
import sys
import sysconfig
import unittest
from test import support
from test.support import import_helper
from contextlib import contextmanager
from test.test_importlib.util import temp_module
Expand Down Expand Up @@ -112,7 +112,7 @@ def test_module_not_found(self):
class WindowsExtensionSuffixTests:
def test_tagged_suffix(self):
suffixes = self.machinery.EXTENSION_SUFFIXES
abi_flags = "t" if sysconfig.get_config_var("Py_GIL_DISABLED") else ""
abi_flags = "t" if support.Py_GIL_DISABLED else ""
Copy link
Contributor

@colesbury colesbury Dec 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vstinner - looks like Windows tests are failing (possibly due to missing import for support:

https://github.com/python/cpython/actions/runs/7090430380/job/19297369118?pr=112709

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, I saw it before. Thanks for the reminder. It should now be fixed.

ver = sys.version_info
platform = re.sub('[^a-zA-Z0-9]', '_', get_platform())
expected_tag = f".cp{ver.major}{ver.minor}{abi_flags}-{platform}.pyd"
Expand Down
4 changes: 1 addition & 3 deletions Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -1224,9 +1224,7 @@ def test_pystats(self):
@test.support.cpython_only
@unittest.skipUnless(hasattr(sys, 'abiflags'), 'need sys.abiflags')
def test_disable_gil_abi(self):
abi_threaded = 't' in sys.abiflags
py_gil_disabled = (sysconfig.get_config_var('Py_GIL_DISABLED') == 1)
self.assertEqual(py_gil_disabled, abi_threaded)
self.assertEqual('t' in sys.abiflags, support.Py_GIL_DISABLED)


@test.support.cpython_only
Expand Down
14 changes: 14 additions & 0 deletions Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1625,6 +1625,17 @@ get_type_module_name(PyObject *self, PyObject *type)
}


#ifdef Py_GIL_DISABLED
static PyObject *
get_py_thread_id(PyObject *self, PyObject *Py_UNUSED(ignored))
{
uintptr_t tid = _Py_ThreadId();
Py_BUILD_ASSERT(sizeof(unsigned long long) >= sizeof(tid));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just use C11 static_assert() now, which has better error messages.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Py_BUILD_ASSERT() doesn't require an error message which makes my life easier, I don't have to write down an error message :-)

return PyLong_FromUnsignedLongLong(tid);
}
#endif


static PyMethodDef module_functions[] = {
{"get_configs", get_configs, METH_NOARGS},
{"get_recursion_depth", get_recursion_depth, METH_NOARGS},
Expand Down Expand Up @@ -1688,6 +1699,9 @@ static PyMethodDef module_functions[] = {
{"restore_crossinterp_data", restore_crossinterp_data, METH_VARARGS},
_TESTINTERNALCAPI_TEST_LONG_NUMBITS_METHODDEF
{"get_type_module_name", get_type_module_name, METH_O},
#ifdef Py_GIL_DISABLED
{"py_thread_id", get_py_thread_id, METH_NOARGS},
#endif
{NULL, NULL} /* sentinel */
};

Expand Down
Loading