Read from pty master in nonblocking mode (due to unlikely deadlock)

[egmontkob]

Is there an existing issue for this?

• I have searched the existing issues

Midnight Commander version and build configuration

4.8.33-353-g32b7a14c4

Operating system

any

Is this issue reproducible using the latest version of Midnight Commander?

• I confirm the issue is still reproducible with the latest version of Midnight Commander

How to reproduce

src/subshell/common.c contains the following typical pattern 3 times (#4625 is about to add 2 more):

• select() on mc_global.tty.subshell_pty, possibly along with other fds
• if other fds were present then check if mc_global.tty.subshell_pty is said to contain data
• read() from this fd

With tty lines, this standard pattern of reading can cause a lockup. That's because the slave side can perform a tcflush(), i.e. revoke the data already placed in the tty line. And this could happen between the select() and the blocking read() steps.

We should set the master fd to O_NONBLOCK (a.k.a. O_NDELAY) mode. Either for the duration of the read()s only, or once and for all but then make sure to adjust the write() / write_all() calls to expect that attempting to write to a clogged channel behaves differently and returns immediately (write_all() would need to be a select()+write() loop rather than just a write() loop).

I guess a helper read_nonblock() method flipping the flag back and forth is the easiest solution for now, until proper glib mainloop-based approach is implemented.

Linux has preadv2() that allows to pass flags on a per-call basis (for some reason it uses RWF_NOWAIT instead of O_NONBLOCK) but it's super non-portable.

Expected behavior

never to block

Actual behavior

super unlikely to block

Additional context

No response

[egmontkob]

I'm trying to come up with a PoC triggering the deadlock :)

[egmontkob]

Here's a reproducer:

mc-4780-trigger-deadlock.patch

This inserts a delay to mc; fair play for debugging a race condition, the scheduler can at any time just randomly decide to introduce a bit of delay there.

Execute this program from mc, on macOS:

flush.c

It hangs mc immediately, no keypresses work, the panels don't return after the program finishes.

If you enable the last fprintf() in the program then it unlocks the deadlock, new data arriving makes that read() finally proceed after 5 seconds. Still, during those 5 seconds, keypresses are not echoed back, showing that they aren't forwarded to the slave.

Not so surprisingly after 4625's investigation, the behavior depends on the platform. I couldn't (yet?) trigger a bug on Linux.

[egmontkob]

Fix: mc-4780-trigger-deadlock.patch

This doesn't touch the bits introduced by 4625. I think this patch should go first, and that one should have the correct code here straight away.

This patch, combined with the test above (including the injected delay) shows that -- depending on timing --, the "some output" string is sometimes flushed while sometimes isn't, but mc no longer hangs.

I'll create a PR in a couple of days tops.