Commit Graph

131 Commits

Author SHA1 Message Date
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
419d7a5138 Don't decompose shared_ptr to raw pointer for exec_job 2018-10-27 18:01:38 -05:00
Mahmoud Al-Qudsi
3afcca3114 Drop keepalive process even for WSL
Windows 10 17763 Redstone 5 (October 2018 Update) officially brings
zombie support (first introduced in 17713) to the general public.

See https://docs.microsoft.com/en-us/windows/wsl/release-notes#build-17763-1809
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
Mahmoud Al-Qudsi
e753581df7 Bring some consistency and rationale to debug log levels
* Debug level 3: describe all commands being executed (this is, after all,
a shell and one can argue that this is the most important debug
information avaliable)
* Debug level 4: details of execution, mainly fork vs no-fork and io
handling

Also introduced j->preview() to print a short descriptor of the job
based on the head of the first process so we don't overwhelm with
needless repitition, but also so that we don't have to rely on
distinguishing between repeated, non-unique/non-monotonic job ids that
are often recycled within a single "execution cycle" (pressing enter
once).
2018-10-27 18:01:38 -05:00
Mahmoud Al-Qudsi
d467bb58d9 Replace pid/pgid -2 with INVALID_PID 2018-10-27 18:01:38 -05:00
Mahmoud Al-Qudsi
af0c8d51e0 Overhaul job and terminal control
* Instead of reaping all child processes when we receive a SIGCHLD, try
reaping only processes belonging to process groups from fully-
constructed jobs, which should eliminate the need for the keepalive
process entirely (WSL's lack of zombies not withstanding) as now
completed processes are not reaped until the job has been fully
constructed (i.e.  all processes launched), which means their process
group should still be around for new processes to join.

* When `tcgetpgrp()` calls return 0, attempt to `tcsetpgrp()` before
invoking failure handling code.

* When forking a builtin and not running interactively, do not bail if
unable to set/restore terminal attributes.

Fixes #4178. Fixes #3805. Fixes #5210.
2018-10-27 18:01:38 -05:00
ridiculousfish
a17a815c87 Revert "Add vector of cleanup/termination events to be executed before quit"
This reverts commit 8c14f0f30f.

This list is not reliable - there are many ways for fish to quit that does not
invoke these functions. It's also not necessary since the history is correctly
saved on exec.
2018-09-28 20:21:23 -04:00
Mahmoud Al-Qudsi
8c14f0f30f Add vector of cleanup/termination events to be executed before quit 2018-09-28 11:34:07 -05:00
ridiculousfish
ca61fc1bf8 Stop retrying close() on EINTR
https://lwn.net/Articles/576478/
http://austingroupbugs.net/view.php?id=529
https://sourceware.org/bugzilla/show_bug.cgi?id=14627
2018-09-05 21:49:31 -07:00
ridiculousfish
8b277e711e Large refactor of exec.cpp
Break up that monster function.
2018-09-03 15:57:11 -07:00
ridiculousfish
eca4d113c6 Factor do_fork into a real function 2018-09-03 14:33:53 -07:00
ridiculousfish
2a62e18635 Remove child_forked and child_spawned
These variables weren't used for anything.
2018-09-03 13:31:03 -07:00
ridiculousfish
f7a020ad33 Rename launch process to exec_process_in_job
This avoids a name collision with another launch_process
2018-09-03 11:18:39 -07:00
ridiculousfish
48c510572b Factor out launch_process from exec.cpp
Makes the monster function slightly more tractable.
2018-09-01 14:54:23 -07:00
ridiculousfish
753639aa9c Reduce the scope of pid in exec_job 2018-09-01 14:39:32 -07:00
ridiculousfish
ec9c592edc Adopt autoclose_fd_t in exec_job 2018-09-01 14:27:58 -07:00
ridiculousfish
d9f34147c3 builtins to only acquire terminal if owned by their pgroup
Fix #5133 changed builtins to acquire the terminal, but this regressed
caused fish to be stopped when running in background via `sudo fish`.
Fix this by only acquiring the terminal if the terminal was owned by the
builtin's pgroup.

Fixes #5147
2018-08-18 16:56:01 -07:00
ridiculousfish
d34a300818 Add string split0
This adds a new string command split0, which splits on zero bytes.
split0 has superpowers because its output is not further split on
newlines when used in command substitutions.
2018-07-01 15:56:33 -07:00
ridiculousfish
f998afaa23 Adopt separated_buffer_t in io_buffer_t 2018-07-01 15:56:33 -07:00
ridiculousfish
90a4af5112 Add separated_buffer_t and adopt it in output_stream_t
separated_buffer_t encapsulates the logic around discarding (which
was previously duplicated between output_stream_t and io_buffer_t),
and will also encapsulate the logic around explicitly separated
output.
2018-07-01 15:56:33 -07:00
ridiculousfish
5b9331ade0 Teach io_buffer_t to append from output_stream_t directly
This will simplify logic when we teach output_stream_t about explicitly
split outputs, i.e. for 'string split0'
2018-07-01 15:56:33 -07:00
ridiculousfish
2443ea92c3 Eliminate a common subexpression 2018-06-16 11:43:52 -07:00
Mahmoud Al-Qudsi
0dd2607cac Iron out situation with setpgid() calls after posix_spawn()
Closes #4715. Ticks off a box in #4154.
2018-05-16 19:34:56 -05:00
Aaron Miller
517b77ca74 Fix handling of signals (#4851) 2018-03-24 12:37:15 -07:00
Mahmoud Al-Qudsi
26cc112096 Implement $last_pid, taking the place of %last
Set as a global variable upon the execution of a background job.
2018-03-09 08:56:13 -06:00
Mahmoud Al-Qudsi
2f2a221c56 Don't spawn keepalive for WSL when only one command
This should speed things up on slower PCs given that the vast majority
of shell commands are simple jobs consisting of a single command without
any pipelines, in which case there's no need for a keepalive process at
all. Applies to WSL only.
2018-03-04 21:54:12 -06:00
Mahmoud Al-Qudsi
cf8850a33f Add temporary fix for #4778 (background processes on WSL)
As a temporary workaround for the behavior described in
Microsoft/WSL#2997 wherein WSL does not correctly assign the spawned
child its own PID as its PGID, explicitly set the PGID for the newly
spawned process.
2018-03-04 20:18:38 -06:00
Mahmoud Al-Qudsi
50541544f2 Distinguish between function and block IO for fork debug log messages 2018-02-18 16:49:27 -06:00
Mahmoud Al-Qudsi
be13ac353b Refactor job control to make functions act like their names imply
The job control functions were a bit messy, in particular
`set_child_group`'s name would imply that all it does is set the child
group, but in reality it used to set the child group (via `setpgid`),
set the job's pgrp if it hasn't been set, and possibly assign control of
the terminal to the newly-created job.

These have been split into separate functions. Now `set_child_group`
does just (and only) that, `maybe_assign_terminal` might assign the
terminal to the new pgrp, and `on_process_created` is used to set the
job properties the first time an external process is created. This might
also speed things up (but probably not noticeably) as there are no more
repeated calls to `getpgrp()` if JOB_CONTROL is not set.

Additionally, this closes #4715 by no longer unconditionally calling
`setpgid` on all new processes, including those created by `posix_spawn`
which does not need this since the child's pgrep is set at in the
arguments to that API call.
2018-02-14 19:08:12 -06:00
ridiculousfish
c3f1961e36 Stop copying out function definition when executing a function
This switches function execution from the function's source code to
its stored node and pstree. This means we no longer have to re-parse
the function every time we execute it.
2018-02-12 10:55:00 -08:00
ridiculousfish
976514597d Migrate function getters to use function_get_properties
This replaces some of the teensy function getters with the function
that just returns a shared_ptr to the properties struct.
2018-02-12 10:53:22 -08:00
ridiculousfish
41ba0dfadb Evaluate tnode_t instead of parse_node_t
This concerns block nodes with redirections, like
begin ... end | grep ...
Prior to this fix, we passed in a pointer to the node. Switch to passing
in the tnode and parsed source ref. This improves type safety and better
aligns with the function-node plans.
2018-02-12 10:51:39 -08:00
ridiculousfish
cb03be9fe6 Remove unused 'pgrp_set' variable 2018-02-07 12:54:26 -08:00
ridiculousfish
8de266afb4 Improve commenting regarding process groups and builtins. 2018-02-07 12:49:12 -08:00
ridiculousfish
0c18f68cc2 Remove support for blocking children
This removes support for blocking children via signals, which was used
to orchestrate processes on WSL. Now we use the keepalive mechanism
instead.
2018-02-07 12:49:12 -08:00
ridiculousfish
080521071f Teach keepalives to exit when their parent dies
keepalive processes are typically killed by the main shell process.
However if the main shell exits the keepalive may linger. In WSL
keepalives are used more often, and the lingering keepalives are both
leaks and prevent the tests from finishing.

Have keepalives poll for their parent process ID and exit when it
changes, so they can clean themselves up. The polling frequency can be
low.
2018-02-07 12:49:12 -08:00
ridiculousfish
e9f676a7f4 Provide a way to stop blocking children via s_block_children
This is to investigate alternatives to the existing kill(SIGSTOP)
WSL compatibility thing.
2018-02-07 12:49:11 -08:00
ridiculousfish
1b1fd5ab9b Mark needs_keepalive more often for WSL
Have WSL use a keepalive whenever the first process is external.
This works around the fact that WSL prohibits setting an exited
process as the group leader.
2018-02-07 12:49:11 -08:00
ridiculousfish
d17b298a48 Factor out the code that executes a builtin from exec_job()
Very early work on untangling the exec_job spaghetti.
2017-12-22 13:41:29 -08:00
Aaron Gyes
a9283803d4 Revert "Non-exported vars: rename SHLVL to shlvl"
Duh, of course it is exported.

This reverts commit 5fc17dcc82.
2017-10-15 04:37:34 -07:00
Aaron Gyes
5fc17dcc82 Non-exported vars: rename SHLVL to shlvl
Fixes #4414
2017-10-15 04:33:27 -07:00
Mahmoud Al-Qudsi
9fdfe44236 Fix type of pid_status variable
We had pid_status defined as a pid_t instance, which was fine since on
most platforms pid_t is an alias for int. However, that is not
universally the case and waitpid takes an int *, not a pid_t *.
2017-09-26 08:16:36 -05:00
ridiculousfish
3d40292c00 Switch env_var to using maybe_t
This eliminates the "missing" notion of env_var_t. Instead
env_get returns a maybe_t<env_var_t>, which forces callers to
handle the possibility that the variable is missing.
2017-09-01 00:14:42 -07:00
Kurtis Rader
f872f25f5b change env_var_t to a vector of strings
Internally fish should store vars as a vector of elements. The current
flat string representation is a holdover from when the code was written
in C.

Fixes #4200
2017-08-18 16:24:30 -07:00
Kurtis Rader
58b604c5ba change order of env_set() args
It's bugged me forever that the scope is the second arg to `env_get()`
but not `env_set()`. And since I'll be introducing some helper functions
that wrap `env_set()` now is a good time to change the order of its
arguments.
2017-08-14 18:18:09 -07:00
Kurtis Rader
975a5bfbde make style-all time again
Recent changes have introduced some style deviations so clean them up.
2017-08-06 16:05:51 -07:00
Kurtis Rader
acdb81bbca lint and style cleanups 2017-08-06 15:47:01 -07:00
Kurtis Rader
083224d1c0 fixes to job control changes
The job control changes need a couple of fixes for compatibility with
changes I merged while @mqudsi was workin on his change.
2017-08-06 15:25:42 -07:00
Kurtis Rader
52d739c746 Revert "Revert "finish cleanup of signal blocking code""
This reverts commit 35ee28ff24.

Reapply the signal blocking cleanup change on top of the job control
changes made by @mqudsi.
2017-08-06 14:46:12 -07:00