On Fri, 10 Dec 2021 12:12:44 +0100 (CET) Johannes Schindelin wrote: > On Fri, 10 Dec 2021, Takashi Yano wrote: > > Could you please test if the following patch solves the issue? > > It does!
It seems that you already apply this patch to msys2, however, this is just an experimental patch to identify the cause of the problem. Please wait a while for actual patch. > However, I am a bit frustrated because there is still a lot light-shedding > to be done. In the current shape of the code, I do not even understand > what it does, let alone why it works around the problem. > > For example, why is there such a long `pcon` stuff going on? I am in the > _disabled_ pseudo console mode, for starters. Like, why is there a > `pcon_input_state`? And why has the `disable_pcon` code path changed at > all (there was no need to touch it, was there)? > > Also, `needs_xfer` clearly means `needs transfer`. What transfer? What's > `masked`? And how does it differ from `mask`? > > I fear that the pseudo console/non-pseudo console code currently has a > lottery factor of 1. I spent a good part of three entire working days > pouring over it, and I still do not understand it. Usually, a combination > of reading the commit messages, reading the code, parsing > function/variable names with a sprinkling of intuition gets me very far in > understanding any kind of legacy code, but not here. And I do _a lot_ of > legacy code hacking, as part of maintaining Git for Windows. The pseudo > console code in Cygwin really is a class of its own in this regard. > > And I have the very strong sense that it does not have to be that way. > > I would really like it if the code in `fhandler_*` could see some tender, > loving care, bringing clarity about, for example clearly distinguishing > between the code paths that use pseudo console support vs not, and code > paths regarding Cygwin processes vs not. > > I mean, even if your diff below is short, I cannot review it. Not the > context, not my study of three days of the surrounding code and the commit > messages, none of that equips me with enough knowledge to even spot an > obvious bug, because such a bug would still not be obvious to me. > > I really hope that this can be fixed. Please let me know if there is > anything I can do to help bring this about. The current pty code is too complicated indeed. :( It is very difficult to explain how it works. Basically, pty has two pairs of pipes, one is for cygwin apps (io_handle/output_handle), the other is for non-cygwin app ( io_handle_nat/output_handle_nat). This is because these pipe- pairs are processed differently even without ConPTY. Outputs to output_handle_nat is forwarded to output_handle by pty_master_fwd_thread after appropriate processing. Input from keyboard is switched between two input pipes, i.e. io_handle and io_handle_nat. When the cygwin-app is activated, input from keyboard is sent to io_handle, and when the non- cygwin app is activated, input from keyboard is sent to io_handle_nat. This switching is done based on switch_to_pcon_in and pcon_input_state. The name of this variable and related function is *pcon*, however, these are also used for non-cygwin apps even without ConPTY by historical reason. While non-cygwin app is activated, switch_to_pcon_in is true and pcon_input_state == to_nat. However, if the cygwin-app is started from non-cygwin app, input from keyboard should be sent to io_handle. This is done using mask_switch_to_pcon_in(). By this function, input is temporary sent to io_handle even if switch_to_pcon_in is true. The function transfer_input() is used to transfer type ahead inupt between two input pipes, i.e. io_handle and io_handle_nat. Masking switch_to_pcon_in state by mask_switch_to_pcon_in() is done in read() and select(), so input while read() or select() is NOT called is sent to io_handle_nat rather than io_handle if switch_to_pcon_in is true. The bug to be fixed now is just the case. So, transferring input from io_handle_nat to io_handle solves the issue in this case. The patch I have sent yesterday is to add this missing action. In addition, this problem does not occur if ConPTY is enabled. -- Takashi Yano <takashi.y...@nifty.ne.jp>
