Ticket #4630 (new defect)
Opened 3 hours ago
Last modified 29 minutes ago
mc doesn't fully resize itself if reading directory while terminal's resized
Reported by: | zaytsev | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | 4.8.34 |
Component: | mc-tty | Version: | 4.8.30 |
Keywords: | Cc: | michael@… | |
Blocked By: | Blocking: | ||
Branch state: | no branch | Votes for changeset: |
Description
Forwarding the bug from Debian - https://bugs-devel.debian.org/cgi-bin/bugreport.cgi?bug=1060651 :
If a terminal window's resized at a "bad time", mc's panels seem to keep
their old sizes indefinitely--even I press CTRL-L or navigate to another
directory. Oddly, the command bar at the bottom of the screen ("1Help",
etc.) does seem to be drawn with the proper width, but not on the proper
line. The rest of the interface doesn't react at all.
I sometimes notice this if I navigate into a large directory, and resize
the window while the "spinner" is animating at the top-right corner. It
can be difficult to reproduce once Linux has the data cached.
I've had some luck using an LD_PRELOAD library that hooks readdir() with
a usleep() call inserted. I'll attach that. To use it:
gcc -shared -o readdir-usleep.so readdir-usleep.c export LD_PRELOAD="$(pwd)/readdir-usleep.so" cd /var/lib # or something with about 50 files mc
When mc clears the screen and starts animating the "/" at the top-right,
enlarge the terminal window. I'm using urxvt (rxvt-unicode).
I'm experimenting with an automated version of this test, but don't know
how reliable it will be.
The attached test case seems able to determine the width of the panels.
Without proper xterm-control and UTF-8 parsing, it may be fragile.
To build and run:
gcc -shared -o readdir-wait.so readdir-wait.c gcc mc-terminal-resize-during-readdir.c ./a.out
Or run "./a.out -P" to skip the $LD_PRELOAD setting, in which case the
signal isn't likely to come during readdir() and we'll measure a width
of 90 columns, as expected. So, I suspect it will print "PASS" if the
bug is fixed, but I don't know for sure.
It looks like the spinner--rotate_dash()--is actually the culprit. That
calls mc_refresh(), which empties sigwinch_pipe before do_nc() makes its
first dlg_run() call:
(gdb) bt #0 tty_flush_winch () at ./lib/tty/tty.c:224 #1 0x00005555556567fb in dialog_change_screen_size () at ./lib/widget/dialog-switch.c:426 #2 0x0000555555656781 in mc_refresh () at ./lib/widget/dialog-switch.c:411 #3 0x000055555558f98e in rotate_dash (show=1) at ./src/filemanager/layout.c:1087 #4 0x000055555559995c in panel_dir_list_callback (state=DIR_READ, data=0x555555704ac0) at ./src/filemanagerpanel.c:4359 #5 0x00005555555fdff7 in dir_list_load (list=0x555555801688, vpath=0x5555557f5020, sort=0x5555555fccec <sort_name>,sort_op=0x555555801760, filter=0x5555558017c0) at ./src/filemanager/dir.c:670 #6 0x000055555559a1b7 in panel_sized_with_dir_new (panel_name=0x5555556784aa "New Right Panel", y=0, x=0, lines=0,cols=0, vpath=0x0) at ./src/filemanager/panel.c:4612 #7 0x000055555558d975 in panel_sized_new (panel_name=0x5555556784aa "New Right Panel", y=0, x=0, lines=0, cols=0) at../../src/filemanager/panel.h:272 #8 0x000055555558ed29 in restore_into_right_dir_panel (idx=1, last_was_panel=0, y=0, x=0, lines=0, cols=0) at ./srcfilemanager/layout.c:668 #9 0x000055555558fca3 in create_panel (num=1, type=view_listing) at ./src/filemanager/layout.c:1189 #10 0x000055555558279a in create_panels () at ./src/filemanager/filemanager.c:658 #11 0x0000555555582ee3 in create_file_manager () at ./src/filemanager/filemanager.c:911 #12 0x0000555555584bed in do_nc () at ./src/filemanager/filemanager.c:1840 #13 0x0000555555570a92 in main (argc=1, argv=0x7fffffffe248) at ./src/main.c:458 (gdb)
It's dlg_run() that will put the file browser into the top_dlg list; but
top_dlg is still empty in the above backtrace, so no MSG_RESIZE messages
are sent. If SIGWINCH comes at the "expected" time, I see this:
(gdb) bt #0 send_message (w=0x5555557f5a80, sender=0x0, msg=MSG_RESIZE, parm=0, data=0x0) at ../../lib/widgetwidget-common.h:250 #1 0x0000555555655e79 in dialog_switch_resize (d=0x5555557f5a80) at ./lib/widget/dialog-switch.c:136 #2 0x00005555556568cc in dialog_change_screen_size () at ./lib/widget/dialog-switch.c:447 #3 0x0000555555657020 in frontend_dlg_run (h=0x5555557f5a80) at ./lib/widget/dialog.c:293 #4 0x0000555555657ac2 in dlg_run (h=0x5555557f5a80) at ./lib/widget/dialog.c:574 #5 0x0000555555584bff in do_nc () at ./src/filemanager/filemanager.c:1841 #6 0x0000555555570a92 in main (argc=1, argv=0x7fffffffe248) at ./src/main.c:458 (gdb)
Simply sticking a "return" at the top of rotate_dash() makes the problem
unreproducible, and gives me a PASS from the test case. Something like
a global 'is_ready' flag might be a better way to do it; or just ensure
dialog_change_screen_size() doesn't flush the pipe if the dialog list is
empty.
While debugging this, I noticed a related bug in toggle_subshell(),
which has this code:
was_sigwinch = tty_got_winch (); tty_flush_winch ();
If a SIGWINCH were handled after setting was_sigwinch to 0 but before
flushing the pipe, the SIGWINCH would be missed. I never saw it happen,
but it could be easily fixed by replacing that code with:
was_sigwinch = tty_flush_winch ();
and making that function return the appropriate boolean value.
Setting g->winch_pending in group_init() also works, and I don't imagine
it would have any negative effect.
Attachments
Change History
comment:1 Changed 41 minutes ago by ossi
thinking big here, the obviously correct solution would be porting mc to the glib main loop, which would properly synchronize signal delivery, thus finally resolving the various race conditions related to sigwinch in particular.
this isn't a trivial project, but it also isn't huge. it might be actually faster to pull it off instead of investigating the issues of the current code.
i recently did that in another project, see here and here for example.
comment:2 Changed 29 minutes ago by zaytsev
I agree with you, and I think Andrew would agree as well. Sadly, such a project is currently not something I could undertake. If someone would like to / could do it, patch with tests would be most welcome.