High CPU usage when running in disowned mode

[nightfriendly]

Important Information

• mpv version

[cplayer] mpv 0.35.1 Copyright © 2000-2023 mpv/MPlayer/mplayer2 projects
[cplayer]  built on Thu Feb  9 22:33:08 CET 2023
[cplayer] FFmpeg library versions:
[cplayer]    libavutil       57.28.100
[cplayer]    libavcodec      59.37.100
[cplayer]    libavformat     59.27.100
[cplayer]    libswscale      6.7.100
[cplayer]    libavfilter     8.44.100
[cplayer]    libswresample   4.7.100
[cplayer] FFmpeg version: 5.1.2

• macOS Version: Ventura 13.4, MacBook Air M2
• Source of the mpv binary or bundle: https://laboratory.stolendata.net/~djinn/mpv_osx/
• If known which version of mpv introduced the problem: problem observed with 0.30.0, 0.34.1.
• Possible screenshot or video of visual glitches: not relevant

Reproduction steps

# Start mpv in zsh' disowned mode
$ ./mpv --no-config *.mkv &!
# CPU is around 30%
$ exit

After exiting the terminal, Activity Monitor reports CPU 110%, Preventing Sleep Yes.

Expected behavior

Closing the terminal that ran mpv should have no impact on the CPU usage, even when paused.

Actual behavior

CPU is above 100%.

Log file

output.txt. Made with -v -v --log-file=output.txt.

This doesn't seem related to #11667.

[low-batt]

Reproduced for me with a build from master.

The terminal thread is calling read which is failing and returning the errro code EIO (5). The code retries unless EBADF or EINVAL is returned:

mpv/osdep/terminal-unix.c

Lines 422 to 424 in 650c53d

	int retval = read(tty_in, &buf.b[buf.len], BUF_LEN - buf.len);
	if (!retval || (retval == -1 && (errno == EBADF || errno == EINVAL)))
	break; // EOF/closed

So the terminal thread is in a tight loop retrying the read call.

[nightfriendly]

Thank you so much for looking into this! So it means:

mpv/osdep/terminal-unix.c

Line 423 in 650c53d

if (!retval || (retval == -1 && (errno == EBADF || errno == EINVAL)))

Should be changed to:

 if (!retval || (retval == -1 && (errno == EBADF || errno == EINVAL || errno == EIO)))

[low-batt]

Maybe. Maybe not. I did confirm that as expected making such a change corrects the CPU usage in the case at hand. One worry is that EIO is a rather generic error that may be returned under other circumstances. Before considering such a fix the issue and change that introduced this code should be reviewed to see if there is a better way to address that issue.

Given that, I did not propose a fix as I thought it best this be handled by developers familiar with the history of problems with this source.

[nightfriendly]

The original logic was introduced in 4b5c3ea#diff-5430e3916d2be16faed7c318993e5f26cefae4a686da8f699af4c67814dbf68eR194 and later modified in 6023843#diff-5430e3916d2be16faed7c318993e5f26cefae4a686da8f699af4c67814dbf68eR405 to fix #5833.

The author of both commits, wm4, does not seem to be around anymore.

man 2 read on MacOS tells us it returns EIO in these cases:

     [EIO]              An I/O error occurred while reading from the file system.

     [EIO]              The process group is orphaned.

     [EIO]              The file is a regular file, nbyte is greater than 0, the starting position is before
                        the end-of-file, and the starting position is greater than or equal to the offset
                        maximum established for the open file descriptor associated with fildes.

On Linux:

EIO           I/O error.  This will happen for example when the process
              is in a background process group, tries to read from its
              controlling terminal, and either it is ignoring or
              blocking SIGTTIN or its process group is orphaned.  It may
              also occur when there is a low-level I/O error while
              reading from a disk or tape.  A further possible cause of
              EIO on networked filesystems is when an advisory lock had
              been taken out on the file descriptor and this lock has
              been lost.  See the Lost locks section of fcntl(2) for
              further details.

It seems therefore pretty normal to stop trying and break out of the loop.

@Akemi, sorry to ping you directly, but we might need your guidance here. If you agree with the proposed solution above, I can prepare a pull request.

[Akemi]

for me this would seem fine at first glance. though this change doesn't only affect macOS but linux too and i am not too familiar with this part of the code.

i think it's best to open a PR and let someone with more linux experience look over it too.

[N-R-K]

Looks like breaking on EIO wasn't a good idea since it breaks terminal input when mpv is foregrounded later on (reported by @guidocella on IRC).

I've added a different fix in #11975 to deal with that. It would be nice if someone on mac can test it out and confirm everything still works fine.