python / cpython

The Python programming language
https://www.python.org
Other
63.83k stars 30.55k forks source link

`PyOS_Readline` crashes in a multi-threaded race #123321

Open bharel opened 3 months ago

bharel commented 3 months ago

Crash report

What happened?

The second breakpoint segfaults on Windows (==access violation).

import asyncio
async def main():
    def inner():
        breakpoint()
        pass
    asyncio.create_task(asyncio.to_thread(inner))
    asyncio.create_task(asyncio.to_thread(inner))

asyncio.ensure_future(main())
asyncio.get_event_loop().run_forever()

Core of the problem is in the myreadline.c module. Implementing a fix...

CPython versions tested on:

3.12, CPython main branch

Operating systems tested on:

Windows

Output from running 'python -VV' on the command line:

No response

Linked PRs

gaogaotiantian commented 3 months ago

Do you actually need asyncio and pdb to repro this? If this is a readline issue with multi-thread, it would be nicer if the regression test is closer to the actual problem. Can you repro this with pure readline and threading? Both pdb and asyncio is complicated enough that dropping them off on a regression test might be helpful in the future.

bharel commented 3 months ago

@gaogaotiantian I completely agree with your insight. The problem is that I've attempted achieving it with only threading and pdb, and for some reason it doesn't reproduce.

Theoretically according to the faulting code it should (and should happen much more often). I'll give it another go.

Haven't thought of testing purely with readline. Didn't notice the separate tests.

bharel commented 3 months ago

By the way, out of interest, do you know why the free threading build fails on altering the execution environment? What does it mean?

ZeroIntensity commented 3 months ago

I'm assuming that this also occurs with asyncio.run? Calling get_event_loop like that is deprecated.

bharel commented 3 months ago

Yeah, I managed to reproduce using input() and threading.

gaogaotiantian commented 3 months ago

That's great. I think you can update the test case and put in the file together with other readline tests. I'm not the expert on free-threaded build so I'm afraid I can't help in that area.

bharel commented 3 months ago

For reference, here's the simplest piece of code that triggers the issue:

import threading

def main():
    def func():
        input()
    thread1 = threading.Thread(target=func)
    thread2 = threading.Thread(target=func)
    thread1.start()
    thread2.start()
    thread1.join()
    thread2.join()
    print("never")

main()
bharel commented 3 months ago

@gaogaotiantian @ZeroIntensity fix is ready.

I appreciate your comments :-)