Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2025-01-16 15:04:05 +00:00
Code Issues Projects Releases Packages Wiki Activity

builtins to sometimes not buffer when writing to a pipe

Browse source 
Prior to this change, if you pipe a builtin to another process, it would
be buffered. With this fix the builtin will write directly to the pipe if
safe (that is, if the other end of the pipe is owned by some external
process that has been launched).

Most builtins do not produce a lot of output so this is somewhat tricky to
reproduce, but it can be done like so:

     bash -c 'for i in {1..500}; do echo $i ; sleep .5; done' |
	   string match --regex '[02468]' |
	   cat

Here 'string match' is filtering out numbers which contain no even digits.
With this change, the numbers are printed as they come, instead of
buffering all the output.

Note that bcfc54fdaa fixed this for the case where the
builtin outputs to stdout directly. This fix extends it to all pipelines
that include only one fish internal process.
This commit is contained in:
ridiculousfish 2021-02-08 13:53:53 -08:00
parent 171d09288b
commit 4b9a096cf2
1 changed files with 21 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
src/exec.cpp
View file

@ -434,8 +434,10 @@ static launch_result_t exec_internal_builtin_proc(parser_t &parser, process_t *p
/// \return an newly allocated output stream for the given fd, which is typically stdout or stderr.
/// This inspects the io_chain and decides what sort of output stream to return.
static std::unique_ptr<output_stream_t> create_output_stream_for_builtin(const io_chain_t &io_chain,
int fd) {
/// If \p piped_output_needs_buffering is set, and if the output is going to a pipe, then the other
/// end then synchronously writing to the pipe risks deadlock, so we must buffer it.
static std::unique_ptr<output_stream_t> create_output_stream_for_builtin(
int fd, const io_chain_t &io_chain, bool piped_output_needs_buffering) {
const shared_ptr<const io_data_t> io = io_chain.io_for_fd(fd);
if (io == nullptr) {
// Common case of no redirections.
@ -452,12 +454,24 @@ static std::unique_ptr<output_stream_t> create_output_stream_for_builtin(const i
}
case io_mode_t::close:
// Like 'echo foo >&-'
return make_unique<null_output_stream_t>();
// TODO: reconsider these.
case io_mode_t::file:
// Output is to a file which has been opened.
return make_unique<fd_output_stream_t>(io->source_fd);
case io_mode_t::pipe:
// Output is to a pipe. We may need to buffer.
if (piped_output_needs_buffering) {
return make_unique<string_output_stream_t>();
} else {
return make_unique<fd_output_stream_t>(io->source_fd);
}
case io_mode_t::fd:
// This is a case like 'echo foo >&5'
// It's uncommon and unclear what should happen.
return make_unique<string_output_stream_t>();
}
DIE("Unreachable");
@ -808,10 +822,10 @@ static launch_result_t exec_process_in_job(parser_t &parser, process_t *p,
}
case process_type_t::builtin: {
std::unique_ptr<output_stream_t> output_stream =
create_output_stream_for_builtin(process_net_io_chain, STDOUT_FILENO);
std::unique_ptr<output_stream_t> errput_stream =
create_output_stream_for_builtin(process_net_io_chain, STDERR_FILENO);
std::unique_ptr<output_stream_t> output_stream = create_output_stream_for_builtin(
STDOUT_FILENO, process_net_io_chain, piped_output_needs_buffering);
std::unique_ptr<output_stream_t> errput_stream = create_output_stream_for_builtin(
STDERR_FILENO, process_net_io_chain, piped_output_needs_buffering);
io_streams_t builtin_io_streams{*output_stream, *errput_stream};
builtin_io_streams.job_group = j->group;