I originally posted an issue and draft fix on cpython but considering the scale I think it would be better to discuss here first.
Also paste the body below:
In the current asyncio REPL, the event loop runs in the main thread, while the interactive console operates in a separate thread as both are running blocking loops. This leads to issues with console input handling, as noted in issue gh-140163.
While this is a MacOS-specific issue, it reveals a defect of the existing implementation. Specifically, the REPL can never receives Ctrl-C event and set the readline status properly, as it does normally. As the above code shows, the outer-most loop catches the KeyboardInterrupt and sets a flag, then tells the REPL to respond. However, the console loop never stops when this happens, because you can’t actually interrupt input() from outside the thread.
So I rethink of the necessity of separate threads and also refer to the implementation of IPython. In a REPL, each cell is executed, and the result is awaited. There doesn’t seem to be a need for a concurrent setup. A simple loop.run_until_complete() will do, as IPython does. At the same time, this can also greatly simplify the existing code.
However, I know I could be very wrong. So please correct me.
I don’t think seperate threads is incorrect here, it just needs switching so that interactive console and signal handling is effectively the only thing in the main thread, and move the evaluation and execution (including the event loop) out of the main thread.
I’m not sure which would be more disruptive here (hopefully neither), but both options do change a small bit of user-observable behavior.
Let me make myself clearer, the problem lies in that the input and eval loop running in a sub-thread, so it can’t respond to main thread events correctly.
In concrete, this is the main loop of interact()
while True:
try:
if more:
prompt = sys.ps2
else:
prompt = sys.ps1
try:
line = self.raw_input(prompt)
except EOFError:
self.write("\n")
break
else:
more = self.push(line)
except KeyboardInterrupt:
self.write("\nKeyboardInterrupt\n")
self.resetbuffer()
more = 0
except SystemExit as e:
if self.local_exit:
self.write("\n")
break
else:
raise e
When it is waiting for user input at line line = self.raw_input(prompt), and receives a Ctrl+C, it can’t go to the correct except block as it does in a normal REPL. Now we have to hack it around by an interrupt() method, but apparently it can’t work in all cases(gh-140163).
Another issue is if you connect remote pdb to the async REPL, the REPL can’t respond to the debugger until user presses enter in it.
Removing threads can avoid such oddities and make the code much cleaner and easier to understand. Therefore, it is still worth the change IMO though the changeset may look big.
It’s true that the traceback frames will look different after the change, but I don’t think that’s users should concern.
I believe swapping which event loop (the console vs the interpreter running the asyncio event loop) is in the main thread resolves that, or at least, that’s been my experience with other multi-event loop applications and I don’t see a reason that wouldn’t be the case here since that puts the handler for keyboard interrupt in the main thread along with input, but I mostly brought it up in case there’s a reason I’m unaware of why the IPython approach wasn’t used here as an alternative, or someone has a case more drastically impacted by using it this way.
I think I agree that the IPython approach is simpler to maintain for this specific use case, but I’m (also) unaware of why it was done this way to begin with, and am wary of Chesterson’s fence.
I also considered swapping the main and child thread, but I found that it has a serious side effect: all the code will run in a child thread, leading to, for example, threading.current_thread() returning different results.
Completely agree on that and that is why I posted this. I am waiting for someone to point out my blind spots and say “Hey, you can’t do this for some (obvious) reasons:”
to me, this doesn’t seem like a serious change. Pretty much the only thing that should care about being in the main thread is signal and console handling code, and you already give up most of the ability to handle signals when using the repl (not even just the async one) as well as when using asyncio.run, but this does seem to have more potential for disruption than the IPython approach with the information I have currently.
I didn’t quite get your concern here. Yes the primary thing is to make REPL run in the main thread, but do we really need another thread for event loop given there won’t be any concurrency in this case? By running both REPL and event loop in the main thread the signals can be sent to the handlers correctly without hack, depending on what is running in the front. So I don’t see anything I “give up” in the change.
I’m not sure. I’m aware of cases where signal handling ends up significantly delayed with the proactor event loop (windows) with asyncio and the default asyncio signal handlers, and had to write a pretty significant workaround for this when I cared about deterministic cleanup and shutdown with an application that had windows users, but I don’t see a reason to care about deterministic shutdown in a repl so long as it does actually shutdown and doesn’t hang. I’m more concerned that knowing edge cases like this exist, and that the original design didn’t go with the simplest option, that we’re potentially missing something that won’t be visible until someone who isn’t a forum regular runs into it with their specific use.
That depends on what you want to do in the REPL. Some (external) code won’t run correctly on anything but the main thread. One example is that GUI-related code on macOS should run on the main thread.