Subshell output lost on window resize under tmux, GNU screen

[yurikhan]

To reproduce:

1. Start xterm. Enable 256-color support:

(xterm)$ set TERM=xterm-256color

2. Start tmux with the following configuration enabling 256-color support:

(xterm)$ cat tmux256.conf
set -g default-terminal "screen-256color"

(xterm)$ tmux -f tmux256.conf

3. Start mc:

(tmux)$ mc

4. Hide panels by pressing Ctrl+O.

5. Generate some subshell output:

(mc)$ ls -al

6. Resize the xterm window.

Expected behavior: shell output remains on screen (suitably padded or truncated to new window size).

Observed behavior: screen is cleared, with cursor remaining at the same position.

Versions:

• Ubuntu 16.04
• tmux 2.1
• mc 4.8.15 compiled with libslang2

$ mc -V
GNU Midnight Commander 4.8.15
Built with GLib 2.47.3
Using the S-Lang library with terminfo database
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, sftpfs, fish
Data types: char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;

[yurikhan]

Cause:

• Resizing the window causes a SIGWINCH sent to the mc process.
• sigwinch_handler sets the flag mc_global.tty.winch_flag to 1.
• The select call in feed_subshell is interrupted.
• feed_subshell notices the winch_flag and calls tty_change_screen_size.
• tty_change_screen_size calls SLsmg_reinit_smg.
• SLsmg_reinit_smg sees that Smg_Mode is currently SMG_MODE_NONE (because it was set that way back in reset_smg that was called when panels were hidden).
• SLsmg_reinit_smg calls SLsmg_init_smg.
• SLsmg_init_smg calls init_smg_for_mode(SMG_MODE_FULLSCREEN).
• init_smg_for_mode calls SLtt_init_video.
• SLtt_init_video sends the smcup terminfo sequence to the terminal.
• SLtt_init_video calls SLtt_init_keypad.
• SLtt_init_keypad sends the smkx terminfo sequence to the terminal and flushes the output buffer. (If $TERM starts with xterm or exactly matches screen, this function returns immediately without flushing the buffer, so the issue will not be visible.)
• The terminal switches to alternate screen and clears it.

[yurikhan]

The patch above adds a check for how == QUIETLY to skip the call to tty_change_screen_size when panels are hidden.

This fixes the problem for me and seems not to introduce any regression with the ncurses build. I have not tested it extensively, though.

[yurikhan]

I should probably add that libslang2 is version 2.3.0 here.

[andrew_b]

Branch: 3639_subshell_output_lost
changeset:d4e95d36aa39db5ca0ce34d47a8d2b94b1771cbe

[zaytsev]

I've pushed amended commit as [a435037bad30c7e5dafbe31c0f3cc38230c459a9], because your commit message formatting is unreadable, do you think you could take that instead?

Otherwise, I can surely vote for the branch, because it looks innocent enough, but generally this kind of patches always makes me feel bad: sadly, we don't have any integration tests to cover this, and not only it's unclear if this will subtly break something or not, but also it's not guaranteed that it will not be broken by some other change in the future :-/

[andrew_b]

Merged to master: [c4a8882b13f3d1bec72193646b1d51c9a963633b].

[yurikhan]

Replying to zaytsev:

this kind of patches always makes me feel bad: sadly, we don't have any integration tests to cover this, and not only it's unclear if this will subtly break something or not, but also it's not guaranteed that it will not be broken by some other change in the future :-/

Apparently it was broken ever since the migration to S-Lang, and nobody noticed or got sufficiently annoyed. I did only because I use a tiling window manager which tends to slightly resize windows every time a new window appears.

[zaytsev-work]

Ha-ha-ha, I did have a bad feeling about this patch, and, sure enough, right after release, I have bisected a very annoying bug as being caused by changeset:c4a888, f%&@*!

So, I have i3 / gnome-terminal on Ubuntu 14.04 here:

1) Switch to an empty workspace
2) Create a new terminal (Super + Enter)
3) Start mc
4) Switch to subshell with Ctrl + O
5) Create a new horizontal split with Super + Enter
6) Start vim in mc subshell

Press Ins repeatedly to enjoy watching vim absolutely confused about the window size and completely unusable. What's more annoying is that an explicit WINCH via Super + F doesn't help it.

@yurikhan, you're saying that you're using a tiling window manager, can you reproduce it?

Not an absolutely critical show-stopper, but unless there is a better solution, I would rather revert it altogether... mc is still losing subshell output in screen / tmux on Ctrl + O, so screen / tmux users are accustomed to constant pain anyways.

[yurikhan]

Replying to zaytsev-work:

enjoy watching vim absolutely confused about the window size and completely unusable.

Let’s minimize your recipe:

1. Start a terminal emulator.
2. Start mc in it.
3. Switch to subshell (Ctrl+O).
4. echo $COLUMNS $LINES
5. Resize the window.
6. echo $COLUMNS $LINES

Expected: $COLUMNS and $LINES track the current window size.

Observed: $COLUMNS and $LINES stay the same.

What's more annoying is that an explicit WINCH via Super + F doesn't help it.

Presumably because the WINCH goes to MC which, because of this patch, does not pass it downstream.

Not an absolutely critical show-stopper, but unless there is a better solution, I would rather revert it altogether...

I will work on a better solution.

mc is still losing subshell output in screen / tmux on Ctrl + O, so screen / tmux users are accustomed to constant pain anyways.

Still losing, with the previous patch? Tell me more. I was intending to end this pain with this and #3640.

[yurikhan]

Please revert changeset:c4a888 and try this one instead.

[yurikhan]

A less invasive attempt

[zaytsev-work]

Hi Yuri, thanks for getting back to me!

Please revert changeset:c4a888 and try this one instead.

I have reverted the old patch and tried the new one, and it seems to fix the problem that I'm observing.

As mentioned above, do you have any idea on how this can be tested in an automated fashion, so that it doesn't get broken next time any "fixing" is done on the SLang layer? One could possibly try to set up integration tests with mc², but it's not merged yet and frankly speaking I have no idea as to if and when this will happen. On the unit test level, one could possibly try to check mock behaviors if the code was encapsulated well enough, but I'm not sure it's even practical at the moment.

Still losing, with the previous patch? Tell me more. I was intending to end this pain with this and #3640.

I'm not exactly sure what the intent of #3640 was, but I understood that it had to do something with cursor position on window resizes, which, apparently, is important for some reason.

In my case, I wasn't losing subshell output upon window resizes, but upon panel flips in screen, even if the window size stays constant. I can reproduce it with 4.8.17 on Ubuntu 14.04 & screen as follows:

1. Start screen
2. Start mc
3. Ctrl+O to hide the panels
4. Run something like ls -als
5. Ctrl+O to show panels
6. Ctrl+O to hide panels
7. Observe that subshell output is gone

This has been around for a very long while, and it seems that there is no change with #3640.

[zaytsev-work]

I have to add that I've just tried tmux, and, to my amazement, I can confirm that it doesn't loose subshell output, be it 4.8.16 or 4.8.17... not sure about cursor position, I haven't got time to check this any more attentively yet.

Maybe I should bit the bullet and ditch screen for tmux, even though last time I tried it, I failed, because tmux felt painfully slow for some reason, and I'm just too used to screen quirks.

[yurikhan]

Replying to zaytsev-work:

As mentioned above, do you have any idea on how this can be tested in an automated fashion, so that it doesn't get broken next time any "fixing" is done on the SLang layer?

Perhaps mock the terminal and possibly the user, and add lots of test cases like:

• Given a terminal in initial state,

when tty_change_screen_size() is called,

then the terminal is still in main screen;

• Given a terminal in alternate screen,

when tty_change_screen_size() is called,

then the terminal is still in alternate screen;

• Given a terminal in alternate screen,

when panels are redrawn,

then the terminal has never received a “save cursor position” sequence;

etc.

Unfortunately I cannot propose a fully ready-made solution for mocking the terminal. I know of this Python library called pyte which might be up to the task, but it would need to be somehow integrated with the test framework. In fact, even a C terminal emulation library would need non-trivial integration.

I'm not exactly sure what the intent of #3640 was, but I understood that it had to do something with cursor position on window resizes, which, apparently, is important for some reason.

It is important because losing the cursor position causes the next page of output to overwrite the last one.

In my case, I wasn't losing subshell output upon window resizes, but upon panel flips in screen, even if the window size stays constant. […] This has been around for a very long while, and it seems that there is no change with #3640.

Is your screen configured for alternate screen switching? By default, it is not; you’d need an altscreen on line in your .screenrc. tmux, on the other hand, does alternate screen out of the box.

[zaytsev]

Is your screen configured for alternate screen switching? By default, it is not; you’d need an altscreen on line in your .screenrc. tmux, on the other hand, does alternate screen out of the box.

Hi Yuri, thanks again for your input!

I've tried it with altscreen on, and screen doesn't loose the output with this patch anymore, also when the window is resized :-) Moreover, out of curiosity, I've also tried 4.8.16, and managed to trigger a segfault after a few resizes o_O You've done a great job!

Unfortunately I cannot propose a fully ready-made solution for mocking the terminal. I know of this Python library called pyte which might be up to the task, but it would need to be somehow integrated with the test framework.

Oh, I see, that's a nice idea. I've also heard of pyte, but I didn't realize that it can be very useful for this purpose.

The integration in the testing framework is the easiest part. Autotools come with a simple test harness out of the box, basically, all you need is a script that returns zero on success, and non-zero on failure. I can certainly hack something up and get Travis to run it.

The most difficult part for me would be to write the test cases themselves :-/ I'm afraid that unless you can lend us a hand, in the end, we'll just have to commit this one as usual, without tests... Oh well.

[yurikhan]

Replying to zaytsev:

Autotools come with a simple test harness out of the box, basically, all you need is a script that returns zero on success, and non-zero on failure.

That looks not very hard but annoyingly fiddly.

What we have is a system consisting of two components, the terminal emulator and the application, connected by an RPC channel with an escape sequence–based protocol over stdin/stdout. Moreover, there are two implementations of the protocol (ncurses and slang) and they do not generate strictly equivalent traffic.

A test case would contain some setup/teardown code on both sides of the channel. Additionally, it would need to perform actions on the application side and observe effects on the terminal side, or vice-versa. Assuming pyte as the virtual terminal emulation library, we need the following sequence:

1. The testing script creates a virtual terminal and performs terminal-side test case setup.
2. The testing script forks off a subprocess with its stdin/stdout connected to the virtual terminal and TERM set to something appropriate.
3. The subprocess performs application-side test case setup.
4. The subprocess performs the main test case action (e.g. tty_change_screen_size()).
5. ???
6. The testing script checks a terminal-side effect (e.g. an smcup sequence has never been received).
7. ???
8. The subprocess performs application-side teardown and exits.
9. The testing script performs terminal-side teardown.
10. The testing script collects the test result.

The ??? in steps 5 and 7 indicate some kind of interprocess synchronization, so that the outer script knows when to check the result and then the subprocess knows when to resume teardown.

Maybe we could hijack stdin/stdout in-process and just analyze the bytes that are written on stdout. Maybe there is a virtual terminal library that is usable from C, also in-process. This would simplify testing because that kind of synchronization wouldn’t be necessary.

[yurikhan]

I might have made an impression that I’m working on automated tests for this ticket. I’m sorry but I’m not. IMO that should be a different ticket.

[andrew_b]

Branch: 3639_subshell_output_lost_fix
Initial changeset:e41401dde118a005dcd627bc286e3e747c0b80c1

[andrew_b]

Merged to master: [57d713085bdb8dfa02a605c99d800b39c15052b9].

git log --pretty=oneline cb06f8c..57d7130

[zaytsev]

Unit test by Michael Gold <michael@…>, I wish we could add this to the check thing...

​https://bugs.debian.org/825974

Still too overwhelmed to try to address it myself :-(