[Midnight Commander] #117: savannah: consecutive resize events not handled correctly

["Ticket System" <tickets midnight-commander org>]

#117: savannah: consecutive resize events not handled correctly
------------------------+---------------------------------------------------
 Reporter:  slavazanko  |       Owner:       
     Type:  defect      |      Status:  new  
 Priority:  major       |   Milestone:       
Component:  mc-core     |     Version:  4.6.1
 Keywords:              |    Blocking:       
Blockedby:              |  
------------------------+---------------------------------------------------
 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.
 }}}

-- 
Ticket URL: <http://www.midnight-commander.org/ticket/117>
Midnight Commander <www.midnight-commander.org>
Midnight Development Center