Commit graph

144 commits

Author SHA1 Message Date
ridiculousfish
ebe2dc2766 Processes to record topic generations before execution
The sigchld generation expresses the idea that, if we receive a sigchld
signal, the generation will be different than when we last recorded it. A
process cannot exit before it has launched, so check the generation count
before process launch. This is an optimization that reduces failing
waitpid calls.
2019-02-17 13:01:59 -08:00
ridiculousfish
a95bc849c5 Rewrite process_mark_finished_children using topics
This is a big change to how process reaping works, reimplenting it using
topics. The idea is to simplify the logic in
process_mark_finished_children around blocking, and also prepare for
"internal processes" which do not correspond to real processes.

Before this change, fish would use waitpid() to wait for a process group,
OR would individually poll processes if the process group leader was
unreapable.

After this change, fish no longer ever calls blocking waitpid(). Instead
fish uses the topic mechanism. For each reapable process, fish checks if
it has received a SIGCHLD since last poll; if not it waits until the next
SIGCHLD, and then polls them all.
2019-02-17 13:01:59 -08:00
Aaron Gyes
717718353e Remove unused macros 2019-02-12 16:10:18 -08:00
ridiculousfish
1701e2c558 Revert "add $pipestatus support"
This reverts commit ec290209db.
2019-02-10 13:46:58 -08:00
ridiculousfish
6da9d96241 Revert "Make $pipestatus thread safe and other misc cleanup"
This reverts commit 34c1f24716.
2019-02-10 13:46:49 -08:00
ridiculousfish
34c1f24716 Make $pipestatus thread safe and other misc cleanup 2019-02-10 13:43:02 -08:00
zabereer
ec290209db add $pipestatus support 2019-02-10 13:30:40 -08:00
ridiculousfish
d3fa58d621 Cleanup common.h
Remove a bunch of headers, simplify lots of code, migrate it into .cpp files.

Debug build time improves by ~3 seconds on my Mac.
2019-02-03 18:22:38 -08:00
ridiculousfish
6f682c8405 Fill io_buffer via background thread
This is a large change to how io_buffers are filled. The essential problem
comes about with code like (example):

    echo ( /bin/pwd )

The output of /bin/pwd must go to fish, not the tty. To arrange for this,
fish does the following:

1. Invoke pipe() to create a pipe.
2. Add an io_bufferfill_t redirection that owns the write end of the pipe.
3. After fork (or equiv), call dup2() to replace pwd's stdout with this  pipe.

Now when /bin/pwd writes, it will send output to the read end of the pipe.
But who reads it?

Prior to this fix, fish would do the following in a loop:

1. select() on the pipe with a 10 msec timeout
2. waitpid(WNOHANG) on the pwd proc

This polling is ugly and confusing and is what is replaced here.

With this new change, fish now reads from the pipe via a background thread:

1. Spawn a background pthread, which select()s on the pipe's read end with
a long (100 msec) timeout.
2. In the foreground, waitpid() (allowing hanging) on the pwd proc.

The big win here is a major simplification of job_t::continue_job() since
it no longer has to worry about filling buffers. This will make things
easier for concurrent execution.

It may not be obvious why the background thread still needs a poll (100 msec).
The answer is for cases where the write end of the fd escapes, in particular
background processes invoked inside command substitutions. psub is perhaps
the only important case of this (other shells typically just hang here).
2019-02-03 01:58:49 -08:00
ridiculousfish
178b72b2fd io_buffer_t becomes io_bufferfill_t
This makes some significant architectual improvements to io_pipe_t and
io_buffer_t.

Prior to this fix, io_buffer_t subclassed io_pipe_t. io_buffer_t is now
replaced with a class io_bufferfill_t, which does not subclass pipe.

io_pipe_t no longer remembers both fds. Instead it has an autoclose_fd_t,
so that the file descriptor ownership is clear.
2019-02-03 01:58:49 -08:00
ridiculousfish
6ba0d4c88a Revert io_bufferfill_t stack
This reverts commit 88dc484858 onwards.
2019-02-02 17:53:40 -08:00
ridiculousfish
9a4153f5e2 Fill io_buffer via background thread
This is a large change to how io_buffers are filled. The essential problem
comes about with code like (example):

    echo ( /bin/pwd )

The output of /bin/pwd must go to fish, not the tty. To arrange for this,
fish does the following:

1. Invoke pipe() to create a pipe.
2. Add an io_bufferfill_t redirection that owns the write end of the pipe.
3. After fork (or equiv), call dup2() to replace pwd's stdout with this  pipe.

Now when /bin/pwd writes, it will send output to the read end of the pipe.
But who reads it?

Prior to this fix, fish would do the following in a loop:

1. select() on the pipe with a 10 msec timeout
2. waitpid(WNOHANG) on the pwd proc

This polling is ugly and confusing and is what is replaced here.

With this new change, fish now reads from the pipe via a background thread:

1. Spawn a background pthread, which select()s on the pipe's read end with
a long (100 msec) timeout.
2. In the foreground, waitpid() (allowing hanging) on the pwd proc.

The big win here is a major simplification of job_t::continue_job() since
it no longer has to worry about filling buffers. This will make things
easier for concurrent execution.

It may not be obvious why the background thread still needs a poll (100 msec).
The answer is for cases where the write end of the fd escapes, in particular
background processes invoked inside command substitutions. psub is perhaps
the only important case of this (other shells typically just hang here).
2019-02-02 14:21:46 -08:00
ridiculousfish
78bbcef356 io_buffer_t becomes io_bufferfill_t
This makes some significant architectual improvements to io_pipe_t and
io_buffer_t.

Prior to this fix, io_buffer_t subclassed io_pipe_t. io_buffer_t is now
replaced with a class io_bufferfill_t, which does not subclass pipe.

io_pipe_t no longer remembers both fds. Instead it has an autoclose_fd_t,
so that the file descriptor ownership is clear.
2019-02-02 14:21:46 -08:00
Mahmoud Al-Qudsi
b54f1842d5 Switch to wait_by_process when waitpid without WNOHANG returns nothing
By exclusively waiting by pgrp, we can fail to reap processes that
change their own pgrp then either crash or close their fds. If we wind
up in a situation where `waitpid(2)` returns 0 or ECHLD even though we
did not specify `WNOHANG` but we still have unreaped child processes,
wait on them by pid.

Closes #5596.
2019-02-02 16:05:57 -06:00
ridiculousfish
a2aab24db7 Switch io_mode to an enum class 2019-01-31 12:12:46 -08:00
ridiculousfish
fec10830d3 Correctly handle exited jobs in process_mark_finished_children
This is effectively a pick of 2ebdcf82ee
and the subsequent fixup. However we also avoid setting WNOHANG unless
waitpid() indicates a process was reaped.

Fixes #5438
2019-01-20 15:07:08 -08:00
ridiculousfish
391af6af0c Introduce class environment_t
This will be used as a base class for variable snapshots and variable stacks.
2019-01-10 20:29:10 -08:00
ridiculousfish
895c2c4af0 Minor cleanup of parser interface 2019-01-10 20:29:10 -08:00
Mahmoud Al-Qudsi
d1913f0df0 Add workaround for Cygwin process management and job control bugs
We cannot wait by pgroup under Cygwin for unknown reasons. Always
wait on jobs by individual processes. See code for more information.
2019-01-02 00:14:07 -06:00
Mahmoud Al-Qudsi
803619b19b Convert some old-school int booleans to bool 2018-12-31 00:46:31 -06:00
Mahmoud Al-Qudsi
0337588979 fixup! Do not use up the ~WNOHANG waitpid call on completed processes 2018-12-30 21:44:14 -06:00
Mahmoud Al-Qudsi
2ebdcf82ee Do not use up the ~WNOHANG waitpid call on completed processes
This is the more correct fix for #5447, as regardless of which process
in the job (be it the first or the last) finished first, once we have
waited on a process without ~WNOHANG we don't do that for any subsequent
processes in the job.

It is also a waste to call into the kernel to wait for a process we
already know is completed!
2018-12-30 20:53:25 -06:00
Mahmoud Al-Qudsi
259cf02aac Wait on individual processes in a job in reverse order
This fixes #5438 by having fish block while waiting on a foreground job
via its individual processes by enumerating the procs in reverse order,
such that we hang waiting for the last job in the IO chain to terminate,
rather than the first.
2018-12-30 19:02:38 -06:00
Mahmoud Al-Qudsi
040d921fa1 Fix check for valid disowned pgids
The function `add_disowned_pgid` adds process *group* ids and not
process ids. It multiplies the value by negative 1 to indicate a wait
on a process group, so the original value must be positive.
2018-12-30 10:15:07 -06:00
Fabian Homborg
4a3ac6e91e Don't wait for disowned pgids if they are special
If a job is disowned that, for some reason, has a pgid that is special
to waitpid, like 0 (process with pgid of the calling process), -1 (any
process), or our actual pgid, that would lead to us waiting for too
many processes when we later try to reap the disowned processes (to
stop zombies from appearing).

And that means we'd snag away the processes we actually do want to
wait for, which would end with us in a waiting loop.

This is tough to reproduce, the easiest I've found was

    fish -ic 'sleep 5 &; disown; set -g __fish_git_prompt_showupstream auto; __fish_git_prompt'

in a git repo.

What we do is to not allow special pgids in the disowned_pids list.
That means we might leave a zombie around (though we probably wait on
0 somewhere), but that's preferable to infinitely looping.

See #5426.
2018-12-30 16:04:57 +01:00
Fabian Homborg
88f7d50633 Acquire terminal even if our pgroup is 0
`tcsetpgrp` still works.

[ci skip]

(This isn't tested)
2018-12-08 16:21:52 +01:00
Fabian Homborg
0a0060481f Guard against pgid == 0
This happens in firejail, and it means that we can't use it as an
argument to most pgid-taking functions.

E.g. `wait(0)` means to wait for the _current_ process group,
`tcsetpgrp(0)` doesn't work etc.

So we just stop doing this stuff and hope it works.

Fixes #5295.
2018-12-08 16:21:52 +01:00
Aaron Gyes
fe67cc4f6e Revert "Show how fish was executed, using argv[0] for program_name"
This reverts commit 1cb8b2a87b.

argv[0] has the full path in it for a user when he executes it
out of $PATH. This is really annoying in the title which uses $_.
2018-11-28 06:08:24 -08:00
Fabian Homborg
047fcb3224 proc: Don't hardcode clr_eol
Also check if that is actually defined, not the cur_term proxy.

In #5371, we figured out that there are terminfo entries without this
capability, so this would do a NULL-dereference.
2018-11-28 13:37:40 +01:00
Aaron Gyes
1cb8b2a87b Show how fish was executed, using argv[0] for program_name
... rather than hard code it to "fish". This affects
what is found in $_ and improves the errors:

For example, if fish was ran with ./fish, instead of
something like:

  fish: Expected 3 surprises, only got 2 surprises

we'll see:

 ./fish: Expected 3 surprises, only got 2 surprises

like most other shell utilities. It's just a tiny bit
of detail that can avoid confusion.
2018-11-27 11:57:09 -08:00
Mahmoud Al-Qudsi
8730b482a7 Prevent zombie processes after disowned child procs exit
Closes #5346.
2018-11-18 15:27:58 -06:00
ridiculousfish
73537fc7c3 Remove NESTED and WAIT_BY_PROCESS
Now jobs are aware of their parent jobs, and can interrogate those jobs,
to determine if every job in the chain is fully constructed.
Remove flags and the static stacks that manipulated them.
2018-11-04 01:52:17 -08:00
ridiculousfish
30990e8069 Replace WAIT_BY_PROCESS with a parent job check
Instead of manipulating the WAIT_BY_PROCESS flag, have each job interrogate
its "parent chain" to decide if it is safe to waitpid() on its pgid.
2018-11-04 01:51:21 -08:00
ridiculousfish
3770d9fb7a Teach each job about its parent
The parent of a job is the parent pipeline that executed the function or
block corresponding to this job. This will help simplify
process_mark_finished_children().
2018-11-04 01:40:07 -08:00
ridiculousfish
93aa95d8c4 Remove proc_last_bg_pid
It wasn't used.
2018-11-03 19:28:16 -07:00
ridiculousfish
0373a87867 Remove shell_pgid from process_mark_finished_children
It was unused.
2018-11-03 19:19:52 -07:00
Mahmoud Al-Qudsi
3b210cc5bd Do not audibly complain on EINTR in waitpid call 2018-10-29 14:22:46 -05:00
Mahmoud Al-Qudsi
132edda6ff Block on fg processes even if not under job control
Closes #5292.
2018-10-29 04:45:47 +00:00
ridiculousfish
fd13043340 Rename select_try_t::IO_ERROR to select_try_t::IOCHAIN_EMPTY
select_try() returned IO_ERROR to indicate that there's no file descriptors
from which to read. Name this return value properly.

Also migrate this type into proc.cpp since it's not used outside of the
header.
2018-10-28 17:14:25 -07:00
ridiculousfish
03ec48c701 Turn the select_try_t into a switch statement
This allows the compiler to warn about missing cases.
2018-10-28 17:12:01 -07:00
ridiculousfish
bf089addd0 Restyle proc.cpp 2018-10-28 17:09:57 -07:00
Mahmoud Al-Qudsi
203de775d0 Fix hang when piping from function to process and exceeding pipe buffer
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.
2018-10-28 10:35:51 -05:00
Mahmoud Al-Qudsi
0d8334a31b Fix hup_background_jobs (née kill_background_jobs) implementation
This was introduced in 1b1bc28c0a but did
not cause any problems until the job control refactor, which caused it
to attempt to signal the calling `exec` builtin's own (invalid) pgrp
with SIGHUP.

Also improved debugging for `j->signal()` failures by printing the
signal we tried sending in case of error, rename the function to
`hup_background_jobs`, and move it from `reader.h`/`reader.cpp` to
`proc.h`/`proc.cpp`.
2018-10-27 18:01:38 -05:00
Mahmoud Al-Qudsi
4d3b56c151 Associate external commands in functions with extant pgrps
When a function is encountered by exec_job, a new context is created for
its execution from the ground up, with a new job and all, ultimately
resulting in a recursive call to exec_job from the same (main) thread.

Since each time exec_job encounters a new job with external commands
that needs terminal control it creates a new pgrp and gives it control
of the terminal (tcsetpgrp & co), this effectively takes control away
from the previously spawned external commands which may be (and likely
are) expecting to still have terminal access.

This commit attempts to detect when such a situation arises by handling
recursive calls to exec_job (which can only happen if the pipeline
included a function) by borrowing the pgrp from the (necessarily still
active) parent job and spawning new external commands into it.

When a parent job spawns new jobs due to the evaluation of a new
function (which shouldn't be the case in the first place), we end up
with two distinct jobs sharing one pgrp (to fix #3952). This can lead to
early termination of a pgrp if finished parent job children are reaped
before future processes in either the parent or future child jobs can
join it.

While the parent job is under construction, require that waitpid(2)
calls for the child job be done by process id and not job pgrp.

Closes #3952.
2018-10-27 18:01:38 -05:00
Mahmoud Al-Qudsi
008eef50f3 Speed up process_mark_finished_children calls
Use SIGCHLD to determine whether or not waitpid(2) calls can be elided,
but only with extreme caution. If we receive SIGCHLD but are not able to
reap all jobs, we need to iterate through them again.

For this to work, we need to make sure that we reap all children that we
can reap after a SIGCHLD, i.e. it's not OK to just reap the first and
return or else we can never clear the dirty state flag.

In all cases, as expensive as a call to waitpid() may be, if a child
process is available for reaping it is always cheaper to wait on it then
reap it than to call select_try() and end up timing out.
2018-10-27 18:01:38 -05:00
Mahmoud Al-Qudsi
319d1b81fb Add note about PROCESS_EXIT still being used 2018-10-27 18:01:38 -05:00
Mahmoud Al-Qudsi
8072900e16 Change control flow in job_continue()
The old code was rather haphazard with regards to error control, and
would make mutable changes before operations that could fail without any
viable error handling options.
2018-10-27 18:01:38 -05:00
Mahmoud Al-Qudsi
39a05a359a Overhaul continue_job() and try_select()
Convert `select_try()` to return a well-defined enum describing its
state, and handle each of the three possible cases with clear reasons
why we are blocking or not blocking in each subsequent call to
`process_mark_finished_children()`.
2018-10-27 18:01:38 -05:00
Mahmoud Al-Qudsi
1bfbed94ae Clean up terminal_give_to_job()
* Use the newly-introduced signal_block_t RAII wrapper
* Remove EINTR loops as all signals are blocked
* Clean up control flow thanks to RAII wrappers
* Rename parameter to clarify what it does and update docs accordingly
* Update outdated comments referencing SIGSTOP code that was removed a
  long time ago.
* Remove no-op CHECK_BLOCK() call
2018-10-27 18:01:38 -05:00
Mahmoud Al-Qudsi
f9118d964e Clean up job flags, status helpers, and instance helper methods
* Convert JOB_* enums to scoped enums
* Convert standalone job_is_* functions to member functions
* Convert standalone job_{promote, signal, continue} to member functions
* Convert standolen job_get{,_from_pid} to `job_t` static functions
* Reduce usage of JOB_* enums outside of proc.cpp by using new
  `job_t::is_foo()` const helper methods instead.

This patch is only a refactor and should not change any functionality or
behavior (both observed and unobserved).
2018-10-27 18:01:38 -05:00