savannah: consecutive resize events not handled correctly

[slavazanko]

Original: ​http://savannah.gnu.org/bugs/?17822

Submitted by:	Egmont Koblinger <egmont>	Submitted on:	Fri 22 Sep 2006 01:04:44 PM UTC
Category:	Screen output	Severity:	3 - Normal
Status:	None	Privacy:	Public
Assigned to:	None	Open/Closed:	Open
Release:	4.6.1	Operating System:	GNU/Linux

Discussion:

Tue 02 Oct 2007 09:18:19 AM UTC, comment #10:

Please, do not steal existing bug reports.
	Pavel Tsekov <ptsekov>
Project Administrator
Mon 01 Oct 2007 09:03:01 PM UTC, comment #9:

when a resize is sent before mc is completely up, it doesn't get the geometry right, either - and yes, this happens about every time an xterm -e mc is restored by session management on my system. so the signal handler should be set up before the initial geometry is queried.
	Oswald Buddenhagen <ossi>
Mon 01 Oct 2007 03:21:44 PM UTC, comment #8:

If you run without mouse support (mc -d), mc doesn't detect resizes.

You need to press a key for mc to resize itself to new window after window is maximized or resized (press up/down arrow, Ctrl-O, almost anything will work).

Tested on 4.6.2-pre1.
	Denis Vlasenko <vda>
Wed 08 Nov 2006 01:38:24 PM UTC, comment #7:

Committed. Quite an improvement in behaviour. Thank you.
	Leonard den Ottolander <leonardjo>
Project Member
Mon 06 Nov 2006 10:24:35 PM UTC, comment #6:

As I understand your patch/hack, eliminating the timeouts on a window resize event significantly reduces the chance mc is still waiting inside the get_event() loop when a new resize event occurs.
	Leonard den Ottolander <leonardjo>
Project Member
Mon 06 Nov 2006 12:01:58 PM UTC, comment #5:

Please commit it if it seems to be okay for you (I've been using mc 4.6.1 with this patch for a month, and found no side effects, while it makes mc much better when resizing the terminal). But please don't yet close this bugreport to remind us that a proper fix is still needed. This patch only makes it much better, but still there's a small chance for the bug to appear. Until we have a proper fix, it's good to apply a temporary hack to heavily decrease the chance for the bug.
	Egmont Koblinger <egmont>
Fri 03 Nov 2006 07:14:55 PM UTC, comment #4:

Shall I commit this patch or should I wait for the piped version?
	Leonard den Ottolander <leonardjo>
Project Member
Tue 10 Oct 2006 04:22:16 PM UTC, comment #3:

Added a resize.patch. This does not suffer from the new bug, though I don't understand what made a difference.

Still a rewrite to using pipes is needed to fill a minor race condition.
	Egmont Koblinger <egmont>
Tue 10 Oct 2006 03:40:51 PM UTC, comment #2:

Oh, I just found the pselect() call which is supposed to solve this problem. However, its manpage says the Linux kernel supports this only since 2.6.16 which is a quite new piece. It says that glibc had an emulation which is vulnerable to the race condition I just mentioned and pselect was just introduced for.
The manpage also mentions and recommends the self-pipe trick which is more portable.
	Egmont Koblinger <egmont>
Tue 10 Oct 2006 03:27:46 PM UTC, comment #1:

Normally mc stays in a select() call in key.c:get_event() which is called by dialog.c:frontend_run_dlg().

frontend_run_dlg() checks if the window size has changed and calls the proper functions in this case. Then it does a lot of other things, then calls get_event() which yet again does a lot of other things and finally arrives at that select().

A subsequent window resize event (SIGWINCH) causes this select to return with -1 EINTR which is later handled correctly.

The problem is caused by the lot of code lines executed after checking for a window size change, but before entering the select call. If another window size change event occurs while executing these commands, it will not be handled until select() exits due to a keypress for example.

In key.c:get_event(), I placed the following line above the "flag = select (...)" statement:
if (winch_flag) return EV_NONE;
and then my resizing problems were gone. So now I check again for a resize event right before entering the select function.

Though this modification seems to solve my bug, another bug appeared. Press F5 on ".." so an error dialog box appears. Resize the window now. mc enters an infinite loop consuming 100% CPU time and not reacting on any keypress. I don't yet know how to fix this new bug. Swapping the new statement and the enable_interrupt_key() call right before it doesn't help either.

On the other hand, even if we didn't have any side effects, my patch still doesn't fix my original problem, only decreases the possibility for this to occur. Still there is a chance that SIGWINCH arrives right after checking for winch_flag but before entering the select() call.

I'm just curious how to solve this problem 100% perfectly, without any race condition, without the slightest possibility to misbehave. Now I'm interested in the theory first, not in the implementation details in mc. Let's suppose a single application that only wants to do two things: process data arriving from several file descriptors, and always correctly display the terminal size from its main loop (i.e. not from a signal handler, since doing complex things from a signal handler is just plain wrong). My only idea is the following: create a pipe, the sigwinch handler writes a byte into it, and the select() call checks for its reading end in addition to the other file descriptors it is interested in. This way the select() call immediately exits if there's an unprocessed sigwinch event, no matter if it occured before or during this select call. And of course we have to set the close-on-exec flag on these fd's so that subprocesses don't inherit them.
	Egmont Koblinger <egmont>
Fri 22 Sep 2006 01:04:44 PM UTC, original submission:

Modern desktop environments and window managers usually resize the windows in an opaque way: plenty of resize events are sent to the application while the edge of the window is being dragged. This gives users a much better feedback than the ancient method (only showing where the edges of the window will be, and resizing when the mouse button is released), though it definitely requires much more cpu resources.

Unfortunately mc is unable to properly handle when the terminal is resized opaquely. It receives tons of resize events (sigwinch), most likely ignores the new ones while processing an older one. Quite often when I finish resizing my window, mc draws its panels with a different size. Hence if I enlarge my terminal, I might get black columns/rows on its right/bottom part. If I shrink the window, the whole screen can be garbled.

Then when I press one key, or click somewhere in the terminal with the mouse, suddenly mc's screen is repainted with the right size, and everything goes on perfectly.

It would be nice to eliminate these race conditions and make mc resize correctly to the final terminal size, no matter how many resize events were sent in a very short time.