labath added a comment.

In D104886#3127411 <https://reviews.llvm.org/D104886#3127411>, @teemperor wrote:

> @labath raised some concerns in IRC about the setup code being run in a 
> potential multithreaded env (which makes sense). Feel free to revert (not 
> sure when/if I'll get around to address the issues)

Yes, I multithreaded code (and code in libraries in particular) should be very 
careful what it's doing with signals and signal handlers. That said, after 
taking a closer look at this. I don't think this patch is as bad as it 
originally seemed to me. The signal handlers are only changed for a bried 
period, and most of the signal code in the patch is there to undo the handlers 
that python sets.

Probably the only thing bothering me now is that all of this work happens 
lazily, whenever we happen to use the python interpreter for the first time. It 
would be much better if this happened in the initialization phase, as most 
applications will likely still be single-threaded at that point (and even if 
they weren't, it any global effects of the SBDebugger::Ininitalize call would 
be less surprising (more predictable)). I wouldn't call that a blocker though, 
since it would require a "how we initialize python" discussion, and it does not 
add any technical debt there (it would be straightforward to move this to the 
initialize call).



================
Comment at: 
lldb/test/API/iohandler/sigint/TestIOHandlerPythonREPLSigint.py:57-59
+    # FIXME: On Linux the Python code that reads from stdin seems to block 
until
+    # it has finished reading a line before handling any queued signals.
+    @skipIfLinux
----------------
This is probably a consequence of (a bug in) our libedit/readline compatibility 
workarounds in `source/Plugins/ScriptInterpreter/Python/PythonReadline.cpp`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104886/new/

https://reviews.llvm.org/D104886

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to