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

Package:
mc
Source:
mc
Description:
Midnight Commander - a powerful file manager
Submitter:
Michael Gold
Date:
2025-02-01 09:09:03 UTC
Severity:
normal
Tags:
#1060651#5
Date:
2024-01-12 03:06:59 UTC
From:
To:
Dear Maintainer,

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.

- Michael

#1060651#10
Date:
2024-01-12 17:47:58 UTC
From:
To:
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.

#1060651#15
Date:
2024-01-13 04:46:56 UTC
From:
To:
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/filemanager/panel.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 ./src/filemanager/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/widget/widget-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.

#1060651#20
Date:
2024-01-14 18:41:31 UTC
From:
To:
Setting g->winch_pending in group_init() also works, and I don't imagine
it would have any negative effect.

#1060651#29
Date:
2025-01-12 11:00:51 UTC
From:
To:
Hi Michael,

Thank you for your excellent research! I have forwarded the bug to upstream tracker.

Unfortunately, Debian doesn’t have the capacity to reliably forward bugs upstream in a timely manner, so to make sure that your reports actually reach us, I would appreciate if we could continue to communicate via our own tracker. Your other bug report was forwarded rather quickly as a matter of luck, and this one I have found by chance while trying to get grasp of what’s going on in Debian while preparing for a release.

I haven’t looked into the details yet, but if you could please provide the patch with a test integrated in our test suite that would be great.

All the best,
Yury

#1060651#34
Date:
2025-01-14 23:37:25 UTC
From:
To:
...

Can you create an account for me?  I don't think I have one, and the
registration page links to a non-existent "captcha.png" image which
I'm assuming is mandatory.  Trying to load it shows a web page saying
"No handler matched request to /captcha.png".

If you're interested, it seems that it might be possible to subscribe to
Debian bug reports via the package tracker:
https://tracker.debian.org/accounts/login/?next=/pkg/mc
(I've never done it, and its documentation link is currently broken, so
I can't tell for sure.  I don't see any way to subscribe to a package
via the bug tracker, which would be more convenient.)

I haven't thought about this in a long time, and don't think I've ever
looked into the integrated test suite.  Maybe one day it'll happen,
though probably not soon.

I remember feeling, like ossi, that there was some sort of larger design
flaw here.  I don't recall whether I considered either of my proposed
changes a proper fix, or understood the code well enough to determine
that.

I did say that my test might not be reliable; so, we may want to figure
out something better for that.  If mc could be made to log certain
diagnostics, for example, that might help (or it might change the timing
so much as to defeat the purpose).

#1060651#39
Date:
2025-01-15 16:16:45 UTC
From:
To:
Interesting, other people seem to be able to register just fine. I have just tried to create an account “michael_gold” for you, hopefully you can reset the password now to gain access to it.

That would be too much for me, I’m barely using Debian these days and I have more mail than I can handle via our upstream tracker.

Oh, I didn’t realize that this bug is now 1 year old :) - I thought it was just created...

Hmmm, I think I’ve missed the point that the test is actually probabilistic. That’s annoying, indeed.

#1060651#52
Date:
2025-02-01 09:04:38 UTC
From:
To:
reopen 1060651
found 1060651 3:4.8.33-1
thanks

Sorry, this will be out in 4.8.34.