mc doesn't fully resize itself if reading directory while terminal's resized

[mc-butler]

Important

This issue was migrated from Trac:

Origin	https://midnight-commander.org/ticket/4630
Reporter	zaytsev (@zyv)
Mentions	michael@….org

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.

Note

Original attachments:

• readdir-wait.c (raw) by zaytsev (@zyv) on Jan 12, 2025 at 10:55 UTC
• mc-terminal-resize-during-readdir.c (raw) by zaytsev (@zyv) on Jan 12, 2025 at 10:55 UTC

[mc-butler]

Changed by zaytsev (@zyv) on Jan 12, 2025 at 10:55 UTC

• Attachment readdir-wait.c (raw) added

[mc-butler]

Changed by zaytsev (@zyv) on Jan 12, 2025 at 10:55 UTC

• Attachment mc-terminal-resize-during-readdir.c (raw) added

[mc-butler]

Changed by ossi (@ossilator) on Jan 12, 2025 at 13:17 UTC (comment 1)

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.

[mc-butler]

Changed by zaytsev (@zyv) on Jan 12, 2025 at 13:29 UTC (comment 2)

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.

[mc-butler]

Changed by andrew_b (@aborodin) on Jan 25, 2025 at 13:01 UTC (comment 3)

• Owner set to andrew_b
• Branch state changed from no branch to on review
• Status changed from new to accepted

Branch: 4630_resize
Initial [bbcfd02b0bc9263234c057ef365db455580eea65]

[mc-butler]

Changed by zaytsev (@zyv) on Jan 25, 2025 at 14:10 UTC (comment 4)

• Branch state changed from on review to approved
• Votes set to zaytsev

master

zaytsev@fedora:~$ ./a.out
initial ws_col: 30
preloaded library stopped in readdir()
setting ws_col: 90
panel width measured as 0 columns
measurer_arg.saw_width, winsz.ws_col: 0, 90
FAIL

4630_resize

zaytsev@fedora:~$ ./a.out 
initial ws_col: 30
preloaded library stopped in readdir()
setting ws_col: 90
panel width measured as 90 columns
measurer_arg.saw_width, winsz.ws_col: 90, 90
PASS

FYI: typos in commit messages - (ialog_change_screen_size) in bbcfd02 and using retuirn value in a42fe0

[mc-butler]

Changed by andrew_b (@aborodin) on Jan 25, 2025 at 18:22 UTC (comment 5)

• Resolution set to fixed
• Branch state changed from approved to merged
• Votes changed from zaytsev to committed-master
• Status changed from accepted to testing

Merged to master: [ed35e73].

git log --oneline e7e21869d..ed35e73ba

[mc-butler]

Changed by andrew_b (@aborodin) on Jan 25, 2025 at 18:23 UTC (comment 6)

• Status changed from testing to closed