Skip to content

Commit 012ea77

Browse files
committed
Merge branch 'pythongh-114271-_thread-ThreadHandle' into pythongh-11427-cherry-pick-ThreadHandle
2 parents 4ed1083 + 9ec6d23 commit 012ea77

File tree

5 files changed

+213
-127
lines changed

5 files changed

+213
-127
lines changed

Include/internal/pycore_lock.h

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ typedef struct {
135135
uint8_t v;
136136
} PyEvent;
137137

138+
// Check if the event is set without blocking. Returns 1 if the event is set or
139+
// 0 otherwise.
140+
PyAPI_FUNC(int) _PyEvent_IsSet(PyEvent *evt);
141+
138142
// Set the event and notify any waiting threads.
139143
// Export for '_testinternalcapi' shared extension
140144
PyAPI_FUNC(void) _PyEvent_Notify(PyEvent *evt);
@@ -158,29 +162,9 @@ typedef struct _PyEventRc {
158162
Py_ssize_t refcount;
159163
} _PyEventRc;
160164

161-
static inline _PyEventRc *
162-
_PyEventRc_New(void)
163-
{
164-
_PyEventRc *erc = (_PyEventRc *)PyMem_RawCalloc(1, sizeof(_PyEventRc));
165-
if (erc != NULL) {
166-
erc->refcount = 1;
167-
}
168-
return erc;
169-
}
170-
171-
static inline void
172-
_PyEventRc_Incref(_PyEventRc *erc)
173-
{
174-
_Py_atomic_add_ssize(&erc->refcount, 1);
175-
}
176-
177-
static inline void
178-
_PyEventRc_Decref(_PyEventRc *erc)
179-
{
180-
if (_Py_atomic_add_ssize(&erc->refcount, -1) == 1) {
181-
PyMem_RawFree(erc);
182-
}
183-
}
165+
_PyEventRc *_PyEventRc_New(void);
166+
void _PyEventRc_Incref(_PyEventRc *erc);
167+
void _PyEventRc_Decref(_PyEventRc *erc);
184168

185169
// _PyRawMutex implements a word-sized mutex that that does not depend on the
186170
// parking lot API, and therefore can be used in the parking lot

Lib/test/test_thread.py

Lines changed: 46 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,8 @@ def task():
189189
with threading_helper.wait_threads_exit():
190190
handle = thread.start_joinable_thread(task)
191191
handle.join()
192-
with self.assertRaisesRegex(ValueError, "not joinable"):
193-
handle.join()
192+
# Subsequent join() calls should succeed
193+
handle.join()
194194

195195
def test_joinable_not_joined(self):
196196
handle_destroyed = thread.allocate_lock()
@@ -233,58 +233,60 @@ def task():
233233
with self.assertRaisesRegex(RuntimeError, "Cannot join current thread"):
234234
raise errors[0]
235235

236-
def test_detach_from_self(self):
237-
errors = []
238-
handles = []
239-
start_joinable_thread_returned = thread.allocate_lock()
240-
start_joinable_thread_returned.acquire()
241-
thread_detached = thread.allocate_lock()
242-
thread_detached.acquire()
236+
def test_join_then_self_join(self):
237+
# make sure we can't deadlock in the following scenario with
238+
# threads t0 and t1:
239+
#
240+
# - t0 joins t1
241+
# - t1 self joins
242+
def make_lock():
243+
lock = thread.allocate_lock()
244+
lock.acquire()
245+
return lock
246+
247+
error = None
248+
self_joiner_handle = None
249+
self_joiner_started = make_lock()
250+
self_joiner_barrier = make_lock()
251+
def self_joiner():
252+
nonlocal error
253+
254+
self_joiner_started.release()
255+
self_joiner_barrier.acquire()
243256

244-
def task():
245-
start_joinable_thread_returned.acquire()
246257
try:
247-
handles[0].detach()
258+
self_joiner_handle.join()
248259
except Exception as e:
249-
errors.append(e)
250-
finally:
251-
thread_detached.release()
260+
error = e
261+
262+
joiner_started = make_lock()
263+
def joiner():
264+
joiner_started.release()
265+
self_joiner_handle.join()
252266

253267
with threading_helper.wait_threads_exit():
254-
handle = thread.start_joinable_thread(task)
255-
handles.append(handle)
256-
start_joinable_thread_returned.release()
257-
thread_detached.acquire()
258-
with self.assertRaisesRegex(ValueError, "not joinable"):
259-
handle.join()
268+
self_joiner_handle = thread.start_joinable_thread(self_joiner)
269+
# Wait for the self-joining thread to start
270+
self_joiner_started.acquire()
260271

261-
assert len(errors) == 0
272+
# Start the thread that joins the self-joiner
273+
joiner_handle = thread.start_joinable_thread(joiner)
262274

263-
def test_detach_then_join(self):
264-
lock = thread.allocate_lock()
265-
lock.acquire()
275+
# Wait for the joiner to start
276+
joiner_started.acquire()
266277

267-
def task():
268-
lock.acquire()
278+
# Not great, but I don't think there's a deterministic way to make
279+
# sure that the self-joining thread has been joined.
280+
time.sleep(0.1)
269281

270-
with threading_helper.wait_threads_exit():
271-
handle = thread.start_joinable_thread(task)
272-
# detach() returns even though the thread is blocked on lock
273-
handle.detach()
274-
# join() then cannot be called anymore
275-
with self.assertRaisesRegex(ValueError, "not joinable"):
276-
handle.join()
277-
lock.release()
278-
279-
def test_join_then_detach(self):
280-
def task():
281-
pass
282+
# Unblock the self-joiner
283+
self_joiner_barrier.release()
282284

283-
with threading_helper.wait_threads_exit():
284-
handle = thread.start_joinable_thread(task)
285-
handle.join()
286-
with self.assertRaisesRegex(ValueError, "not joinable"):
287-
handle.detach()
285+
self_joiner_handle.join()
286+
joiner_handle.join()
287+
288+
with self.assertRaisesRegex(RuntimeError, "Cannot join current thread"):
289+
raise error
288290

289291

290292
class Barrier:

Lib/threading.py

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -932,13 +932,10 @@ def _after_fork(self, new_ident=None):
932932
self._ident = new_ident
933933
if self._handle is not None:
934934
assert self._handle.ident == new_ident
935-
if self._join_lock is not None:
936-
self._join_lock._at_fork_reinit()
937935
else:
938936
# This thread isn't alive after fork: it doesn't have a tstate
939937
# anymore.
940938
self._done_event.set()
941-
self._join_lock = None
942939
self._handle = None
943940

944941
def __repr__(self):
@@ -970,8 +967,6 @@ def start(self):
970967
if self._started.is_set():
971968
raise RuntimeError("threads can only be started once")
972969

973-
self._join_lock = _allocate_lock()
974-
975970
with _active_limbo_lock:
976971
_limbo[self] = self
977972
try:
@@ -1099,17 +1094,9 @@ def join(self, timeout=None):
10991094
self._join_os_thread()
11001095

11011096
def _join_os_thread(self):
1102-
join_lock = self._join_lock
1103-
if join_lock is None:
1104-
return
1105-
with join_lock:
1106-
# Calling join() multiple times would raise an exception
1107-
# in one of the callers.
1108-
if self._handle is not None:
1109-
self._handle.join()
1110-
self._handle = None
1111-
# No need to keep this around
1112-
self._join_lock = None
1097+
# self._handle may be cleared post-fork
1098+
if self._handle is not None:
1099+
self._handle.join()
11131100

11141101
@property
11151102
def name(self):
@@ -1377,6 +1364,10 @@ def __init__(self):
13771364
with _active_limbo_lock:
13781365
_active[self._ident] = self
13791366

1367+
def _join_os_thread(self):
1368+
# No ThreadHandle for main thread
1369+
pass
1370+
13801371

13811372
# Helper thread-local instance to detect when a _DummyThread
13821373
# is collected. Not a part of the public API.

0 commit comments

Comments
 (0)