github-mirror/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2025-01-22 12:44:57 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Fix hang when piping from function to process and exceeding pipe buffer

Browse Source 
This is an opposite case from the usual "pipe into grep-the-function"
where my `pbpaste` emitted a lot of content exceeding the OS pipe
buffer. The `block_on_fg` condition was just `send_sigcont` in the
original job control rewrite, and it was incorrect to sub it for
WAIT_BY_PROCESS on its own.

However, this requires always blocking when select_try returns an
interrupted/incomplete read or else fish doesn't block and stays running
in a tight loop in the background (and incorrectly writing to a terminal
it doesn't own under higher debug levels), which I *think* is OK.
This commit is contained in:
Mahmoud Al-Qudsi 2018-10-28 07:24:10 -05:00
parent 1015e74480
commit 203de775d0
1 changed files with 8 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/proc.cpp
View File

@ -1063,10 +1063,11 @@ void job_t::continue_job(bool send_sigcont) {
// If this is a child job and the parent job is still under construction (i.e. job1 | // If this is a child job and the parent job is still under construction (i.e. job1 |
// some_func), we can't block on execution of the nested job for `some_func`. Doing // some_func), we can't block on execution of the nested job for `some_func`. Doing
// so can cause hangs if job1 emits more data than fits in the OS pipe buffer. // so can cause hangs if job1 emits more data than fits in the OS pipe buffer.
// The test for block_on_fg is this->parent_job.is_constructed(), which coincides // The solution is to to not block on fg from the initial call in exec_job(), which
// with WAIT_BY_PROCESS (which will have to do since we don't store a reference to // is also the only place that send_sigcont is false. parent_job.is_constructed()
// the parent job in the job_t structure). // must also be true, which coincides with WAIT_BY_PROCESS (which will have to do
bool block_on_fg = !get_flag(job_flag_t::WAIT_BY_PROCESS); // since we don't store a reference to the parent job in the job_t structure).
bool block_on_fg = send_sigcont && !get_flag(job_flag_t::WAIT_BY_PROCESS);
// Wait for data to become available or the status of our own job to change // Wait for data to become available or the status of our own job to change
while (!reader_exit_forced() && !is_stopped() && !is_completed()) { while (!reader_exit_forced() && !is_stopped() && !is_completed()) {
@ -1096,8 +1097,9 @@ void job_t::continue_job(bool send_sigcont) {
debug(3, L"select_try: interrupted read from job file descriptors" ); debug(3, L"select_try: interrupted read from job file descriptors" );
// This tends to happen when the foreground process has changed, e.g. it was // This tends to happen when the foreground process has changed, e.g. it was
// suspended and control has returned to the shell. // suspended and control has returned to the shell or when a fg process takes
process_mark_finished_children(block_on_fg); // initial control of the shell.
process_mark_finished_children(true);
// If it turns out that we encountered this because the file descriptor we were // If it turns out that we encountered this because the file descriptor we were
// reading from has died, process_mark_finished_children() should take care of // reading from has died, process_mark_finished_children() should take care of