friss added a comment.

In D79654#2029131 <https://reviews.llvm.org/D79654#2029131>, @labath wrote:

> Yes, resizing the window (or doing any other nontrivial task) from within a 
> signal handler is a bad idea. Making a note of the signal and then bailing 
> out is the right approach. Though, to be fully strictly correct, the variable 
> ought to be a `volatile std::sig_atomic_t` instead of a plain bool. This 
> still won't make the whole thing async-signal-safe (I haven't inspected the 
> whole SIGWINCH call stack, but I am sure there are still some unsafe 
> operations there), but it's a step towards that.


I updated the patch with this change. I'll commit it tomorrow unless you can 
explain this:

> If we wanted to avoid delaying the change to the next keystroke, we could 
> reuse the same mechanism that ^C/SIGINT uses 
> (`m_input_connection.InterruptRead()`). That would probably require 
> introduction of a new `EditorStatus` enum, and a careful modification to the 
> code handling the input interruption. I don't expect that to be _too_ hard, 
> but I also don't think that's required for this change.

I gave this a try, and indeed the changes do not seem too involved, but there's 
a bunch of details I don't get. `Editline::Interrupt()` and 
`Editline::Cancel()` take `m_output_mutex` before interrupting. If I do a 
similar thing in a resize handler, I don't see what prevents the signal to be 
delivered while the lock is held by `GetLines` but before it is released in 
`GetCharacter`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79654



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

Reply via email to