Segfault on Python 3.13.0rc3 (free-threaded) with requests
#124984
Labels
3.13
bugs and security fixes
3.14
new features, bugs and security fixes
extension-modules
C modules in the Modules dir
topic-free-threading
topic-SSL
type-crash
A hard crash of the interpreter, possibly with a core dump
Comments
AA-Turner
added
the
type-crash
AA-Turner
changed the title
requests.requests
|
I ran this in CI to test TSan (TSan archive): |
AA-Turner
changed the title
requestsrequests
AA-Turner
added
interpreter-core
|
Running on a debug build (GHA log): |
|
Here's a reproducer without third-party packages: import queue
import ssl
import threading
class HyperlinkAvailabilityCheckWorker(threading.Thread):
def __init__(self, ctx, wqueue) -> None:
self.ctx = ctx
self.wqueue = wqueue
super().__init__(daemon=True)
def run(self) -> None:
self.wqueue.get()
for _ in range(10000):
self.ctx.set_alpn_protocols(["h2"])
def test_crash2():
for i in range(1_000):
print(f"loop: {i}")
# setup
ctx = ssl.create_default_context()
wqueue = queue.Queue()
workers: list[HyperlinkAvailabilityCheckWorker] = []
# invoke threads
num_workers = 10
for _ in range(num_workers):
thread = HyperlinkAvailabilityCheckWorker(ctx, wqueue)
thread.start()
workers.append(thread)
for _ in range(num_workers):
wqueue.put(None)
# check
for thread in workers:
thread.join()
if __name__ == "__main__":
import sys
print(f"GIL enabled?: {sys._is_gil_enabled()}")
print()
test_crash2() |
ZeroIntensity
added
topic-SSL
extension-modules
|
#124993 should (hopefully) fix all the thread safety issues in |
vstinner
pushed a commit
that referenced
this issue
Oct 19, 2024
|
I modified the reproducer to use more threads: import queue
import threading
import requests
class HyperlinkAvailabilityCheckWorker(threading.Thread):
def __init__(self, rqueue, wqueue) -> None:
self.rqueue = rqueue
self.wqueue = wqueue
self._session = requests.Session()
super().__init__(daemon=True)
def run(self) -> None:
while True:
uri = self.wqueue.get()
if not uri:
self._session.close()
break
self._session.request(
'HEAD',
url=uri,
timeout=30,
verify=True,
)
self.rqueue.put(uri)
self.wqueue.task_done()
def test_crash():
for i in range(1_000):
print(f'loop: {i}')
# setup
rqueue = queue.Queue()
wqueue = queue.Queue()
workers: list[HyperlinkAvailabilityCheckWorker] = []
# invoke threads
num_workers = 10
for _ in range(num_workers):
thread = HyperlinkAvailabilityCheckWorker(rqueue, wqueue)
thread.start()
workers.append(thread)
# check
total_links = 0
for hyperlink in (
'https://python.org/dev/',
'https://peps.python.org/pep-0008/',
'https://peps.python.org/pep-0600/',
'https://peps.python.org/pep-0601/',
'https://peps.python.org/pep-0602/',
'https://peps.python.org/pep-0603/',
'https://peps.python.org/pep-0604/',
'https://peps.python.org/pep-0605/',
'https://peps.python.org/pep-0606/',
'https://peps.python.org/pep-0607/',
):
wqueue.put(hyperlink, False)
total_links += 1
done = 0
while done < total_links:
result = rqueue.get()
print(result)
done += 1
# shutdown_threads
wqueue.join()
for _worker in workers:
wqueue.put('', False)
if __name__ == '__main__':
import sys
print(f'GIL enabled?: {sys._is_gil_enabled()}')
print()
test_crash()Using this modified script, I can easily reproduce the crash. With the fix 4c53b25 I can no longer reproduce the crash (I stopped the script at the 500th iteration). I confirm that the change fixed the requests crash. Thanks @AA-Turner to the bug report! |
ZeroIntensity
added a commit
to ZeroIntensity/cpython
that referenced
this issue
Oct 21, 2024
Make SSL objects thread safe in Free Theaded build by using critical sections. (cherry picked from commit 4c53b25) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
Is there any plan to resolve the missing docstrings? I prefer them to be kept at least as comments in |
|
Oh wait, I guess some of the docstrings did get stripped in the transition over to AC in that PR. Is that worth trying to fix? |
|
@ZeroIntensity yes, we should make sure docstrings are preserved. |
|
I'll deal with that sometime in the next few days then. |
Yhg1s
pushed a commit
that referenced
this issue
Dec 2, 2024
Make SSL objects thread safe in Free Theaded build by using critical sections. (cherry picked from commit 4c53b25) Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
Now that the 3.13 backport is in, I consider this done. Thanks to everyone who reviewed! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Crash report
What happened?
Reproducer:
Sample output:
@JelleZijlstra ran this on macOS with the following lldb traceback:
CPython versions tested on:
3.13
Operating systems tested on:
Linux, macOS
Output from running 'python -VV' on the command line:
Python 3.13.0rc3+ experimental free-threading build (main, Oct 4 2024, 08:50:03) [GCC 11.4.0]
Linked PRs
sslthread safety #124993sslthread safety (GH-124993) #125780The text was updated successfully, but these errors were encountered: