Page MenuHomeFreeBSD

script(1): work around slow reading child
ClosedPublic

Authored by kib on Jan 8 2022, 1:25 PM.
Tags
None
Referenced Files
F103223024: D33789.id101134.diff
Fri, Nov 22, 9:33 AM
Unknown Object (File)
Tue, Nov 19, 11:48 PM
Unknown Object (File)
Tue, Nov 19, 11:46 PM
Unknown Object (File)
Tue, Nov 19, 11:22 PM
Unknown Object (File)
Tue, Nov 19, 11:03 PM
Unknown Object (File)
Sun, Nov 17, 5:29 PM
Unknown Object (File)
Sun, Nov 17, 5:47 AM
Unknown Object (File)
Sat, Nov 16, 11:09 AM
Subscribers

Details

Summary
If child is slow reading from its input, or even completely stops doing
the read, script(1) hangs in write(2) to the pts master waiting until
there is a space in the terminal discipline buffer.  This also stops
handling any outer io, as well as child output.

Work around the problem by making pts master fd non-blocking, and be
prepared for short writes to it.  The data to be written to master is
buffered in the tailq which is processed when select(2) detects that
master is ready for write.

PR:     260938

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Jan 8 2022, 1:25 PM

Seems reasonable to me.

usr.bin/script/script.c
311

I think this can leave the terminal in a weird state, we should call done() before exiting, same way read() errors above are handled.

This revision is now accepted and ready to land.Jan 8 2022, 4:45 PM
kib marked an inline comment as done.Jan 9 2022, 2:01 AM
kib added inline comments.
usr.bin/script/script.c
311

I added done() call, but I in fact do not see what the weird state would be. For the terminal, done() only flushes the pending output.

Also, I do not believe that for correctly operating program, the cc == -1 case there, except for EWOULDBLOCK/EINTR errnos, is ever possible. It can be artificially created, by e.g. revoking the pts in other process. But we cannot do anything useful there then.

kib marked an inline comment as done.

A lot of bug fixes, sorry.

Handle EINTR as acceptable error from write() on master.
Properly use select, i.e. do not forget to zero write set, and then pass it to select.
Fix fcntl(F_SETFL).

This revision now requires review to proceed.Jan 9 2022, 2:02 AM

Fix fcntl(F_GETFL) use even more.

Remove ignored argument for fcntl.

markj added inline comments.
usr.bin/script/script.c
311

script puts STDIN_FILENO in raw mode. done() restores the terminal state.

This revision is now accepted and ready to land.Jan 10 2022, 3:26 PM
This revision was automatically updated to reflect the committed changes.