Commit graph

74 commits

Author SHA1 Message Date
Mahmoud Al-Qudsi
f4ae69a905 fixup! Recover from bad redirections in the middle of a job pipeline
Fix inadvertent early abort (thanks, nested switch-in-for-loop!) that
led to subsequent shell input being broken.
2020-05-30 10:37:46 -05:00
Mahmoud Al-Qudsi
bc756a981e Recover from bad redirections in the middle of a job pipeline
Currently fish aborts execution mid-pipeline if a file redirection
failed, which can leave the shell in a broken state (job abandoned after
giving control of the terminal to an already-executed job in the
pipeline).

This patch replaces a failed fd with a closed fd and continues execution
if the affected process wasn't the first in the pipeline.

While this is a hack to address the regression behind fish-shell/#7038
introduced in d62576c, it can also be argued that this behavior is
actually more correct... right?

Closes #7038.
2020-05-30 00:27:11 -05:00
Rosen Penev
0668513138 Change C casts to C++ ones
Some were kept for compatibility.

Found with -Wold-style-cast

Signed-off-by: Rosen Penev <rosenp@gmail.com>
2020-05-01 13:30:56 -07:00
ridiculousfish
a1f1b9c2d9 builtin_eval to direct output to its iostreams
Prior to this fix, builtin_eval would direct output to the io_chain of the
job. The problem is with pipes: `builtin_eval` might happily attempt to
write unlimited output to the write end of a pipe, but the corresponding
reading process has not yet been launched. This results in deadlock.

The fix is to buffer all the output from `builtin_eval`. This is not fun
but the best that can be done until we have real concurrent processes.

Fixes #6806
2020-04-26 11:05:50 -07:00
ridiculousfish
feb40f0cd6 Make io_file_t::print more useful 2020-04-25 20:25:28 -07:00
Rosen Penev
220f0a132d [clang-tidy] use auto when casting
Found with modernize-use-auto

Signed-off-by: Rosen Penev <rosenp@gmail.com>
2020-04-05 10:13:13 +02:00
ridiculousfish
a765026c4c Adopt fd_monitor in bufferfill
This switches bufferfills from using an exclusively-owned thread, to
sharing an fd_monitor. This allows multiple bufferfills to all use the same
thread.
2020-02-05 12:05:39 -08:00
Fabian Homborg
3bb15defbb
Replace debug() with flog
PR #6511 

Flog has the advantage of having *categories*, not severities, so it'll be easier to get output for a certain subsystem now.
2020-01-26 14:13:17 +01:00
ridiculousfish
4f205f38b4 Clean up a few bits about discarding buffers
We weren't properly propagating the 'discarded' stuff from output
streams to buffers. Fix that.
2020-01-24 16:08:56 -08:00
Fabian Homborg
a48926dee5 Add and use "should_flog" macro
Useful to figure out if a flog category is enabled.

We only use it in one place, but it seems like the sort of thing that
should exist.
2020-01-19 14:55:08 +01:00
Fabian Homborg
024e03ab1e Replace debug(1) with FLOGF(warning) 2020-01-19 14:22:39 +01:00
ridiculousfish
c14d54032f Add a cant_wait parameter to iothread_perform
Sometimes we must spawn a new thread, to avoid the risk of deadlock.
Ensure we always spawn a thread in those cases. In particular this
includes the fillthread.
2020-01-18 11:51:13 -08:00
Fabian Homborg
033a832687
Merge pull request #6447 from neheb/clang2
Several more small clang-tidy cleanups
2019-12-31 18:47:24 +01:00
ridiculousfish
c963442999 Collapse io_data switch statements
Now that each io_data knows its source and target fd, we don't need to switch
on its types any more.
2019-12-29 15:51:22 -08:00
ridiculousfish
0af5608ce8 io_data_t to store the source_fd directly
Now that all io_data_ts know their source fd, just store it directly in
the base class. This will simplify some uses of io_data_t.
2019-12-29 15:14:08 -08:00
ridiculousfish
d0cefe8b65 Always mark pipes as cloexec
There is never a reason to keep these open in exec.
2019-12-29 14:57:16 -08:00
Rosen Penev
9e42b0100a
[clang-tidy] Remove redudant .get on smart pointer
Found with readability-redundant-smartptr-get
2019-12-26 21:25:11 -08:00
ridiculousfish
2e7cbaeaba Remove io_file_t::is_dev_null
This is no longer used.
2019-12-20 14:47:54 -08:00
ridiculousfish
0531c02ce4 Remove 'user_supplied' flag for io_fd_t
user_supplied was used to distinguish IO redirections which were
explicit, vs those that came about through "transmogrphication." But
transmogrification is no more. Remove the flag.
2019-12-19 14:14:23 -08:00
ridiculousfish
d6c71d77a9 Correctly cloexec file redirections
The IO cleanup left file redirections open in the child. For example,
/bin/cmd < file.txt would redirect stdin but also leave the file open.
Ensure these get closed properly.
2019-12-13 16:16:19 -08:00
ridiculousfish
1f83fb47ce Finish the IO cleanup.
Remove some dead code and add missing dtors.
2019-12-12 17:47:08 -08:00
ridiculousfish
33aff87c10 Switch io_file_t to store an fd, not a path
Prior to this fix, a file redirection was turned into an io_file_t. This is
annoying because every place where we want to apply the redirection, we
might fail due to open() failing. Switch to opening the file at the point
we resolve the redirection spec. This will simplify a lot of code.
2019-12-12 17:34:40 -08:00
ridiculousfish
af473d4d0c Introduce redirection_spec_t
Prior to this change, a process after it has been constructed by
parse_execution, but before it is executed, was given a list of
io_data_t redirections. The problem is that redirections have a
sensitive ownership policy because they hold onto fds. This made it
rather hard to reason about fd lifetime.

Change these to redirection_spec_t. This is a textual description
of a redirection after expansion. It does not represent an open file and
so its lifetime is no longer important.

This enables files to be held only on the stack, and are no longer owned
by a process of indeterminate lifetime.
2019-12-12 16:44:24 -08:00
ridiculousfish
be685faeb8 Clean up how pipe fd avoidance works
fish has to ensure that the pipes it creates do not conflict with any
explicit fds named in redirections. Switch this code to using
autoclose_fd_t to make the ownership logic more explicit, and also
introduce fd_set_t to reduce the dependence on io_chain_t.
2019-12-12 14:58:18 -08:00
ridiculousfish
1e15aa4c4a Correctly style io_chain_t::print 2019-12-10 17:21:03 -08:00
ridiculousfish
0573e95b03 Resurrect io_print 2019-12-10 17:18:37 -08:00
ridiculousfish
b5d0075406 Use iothread pool for background fillthreads
Background fillthreads are used when we want to populate a buffer from an
external command. The most common is command substitution.

Prior to this commit, fish would spin up a fillthread whenever required.
This ended up being quite expensive.

Switch to using the iothread pool instead. This enables reusing the same
thread(s), which prevents needing to spawn new threads. This shows a big
perf win on the alias benchmark (766 -> 378 ms).
2019-11-27 12:03:56 -08:00
Rosen Penev
1055ff321c [clang-tidy] Replace NULL with nullptr
Found with modernize-use-nullptr

Signed-off-by: Rosen Penev <rosenp@gmail.com>
2019-11-25 14:23:03 -08:00
ridiculousfish
9023c2187f Make shutdown_fillthread_ a relaxed_atomic_bool_t
Reduces the noisiness of working with it.
2019-11-23 14:12:34 -08:00
ridiculousfish
ac2eed2ffa Make io_chain_t store const io_data_t
This will make it easier to reason about with concurrent execution.
2019-11-10 14:00:30 -08:00
ridiculousfish
521d0e84f5 Remove non-const get_io_for_fd
These could be made unused.
2019-11-10 13:41:12 -08:00
ridiculousfish
424c56006d Add a paranoid assert to io_chain_t::append 2019-11-10 13:31:47 -08:00
ridiculousfish
f2093aef43 Remove io_chain_t::push_front
It was unused.
2019-11-10 13:25:55 -08:00
ridiculousfish
82eca4bc86 Run clang-format on all files
The main change here is to reorder headers.
2019-10-13 15:50:48 -07:00
ridiculousfish
554ee240b3 Correct handling of explicitly separated output when all elements are empty
Previously when propagating explicitly separated output, we would early-out
if the buffer was empty, where empty meant contains no characters. However
it may contain one or more empty strings, in which case we should propagate
those strings.

Remove this footgun "empty" function and handle this properly.

Fixes #5987
2019-07-21 14:00:27 -07:00
ridiculousfish
6ce85aebc6 Switch file_io_t to store a wcstring
We no longer use file_io_t after fork(). We don't need to use a malloc'd
string any more. Use a wcstring.
2019-06-09 17:43:25 -07:00
Fabian Homborg
c2970f9618 Reformat all files
This runs build_tools/style.fish, which runs clang-format on C++, fish_indent on fish and (new) black on python.

If anything is wrong with the formatting, we should fix the tools, but automated formatting is worth it.
2019-05-05 12:09:25 +02:00
Mahmoud Al-Qudsi
ab1519acef Fix high CPU usage in subsequent select(2) calls
The timeout was being reset to zero, so `select` was being called in a
very tight loop.

Closes #5761.
2019-04-09 21:10:30 -05:00
Aaron Gyes
aaacdb89b6 Switches over to cstring from string.h. 2019-03-12 15:09:36 -07:00
Aaron Gyes
d5ac239f68 This commit changes wchar.h includes to cwchar, and uses std::
for everything it provides.
2019-03-12 15:09:36 -07:00
ridiculousfish
0b3eca1743 Cleanup handle_builtin_output
Now that we use an internal process to perform builtin output, simplify the
logic around how it is performed. In particular we no longer have to be
careful about async-safe functions since we do not fork.

Also fix a bunch of comments that no longer apply.
2019-02-17 14:17:44 -08:00
ridiculousfish
ada8ea954e Use "internal" processes to write buffered output
This introduces "internal processes" which are backed by a pthread instead
of a normal process. Internal processes are reaped using the topic
machinery, plugging in neatly alongside the sigchld topic; this means that
process_mark_finished_children() can wait for internal and external
processes simultaneously.

Initially internal processes replace the forked process that fish uses to
write out the output of blocks and functions.
2019-02-17 13:05:20 -08:00
Fabian Homborg
c0dc1870f0 io: Return from read even if return == -1 and errno == 0
This happens on OpenIndiana/Solaris/Illumos/SunOS.

Elsewhere we use read_blocked, which already returned in this
case (and which we might want to use here as well!).
2019-02-13 20:15:09 +01:00
Fabian Homborg
7508865374 Include string.h where we use memset
This is needed on Solaris/Illumos/OpenIndiana/SunOS.

Presumably it's harmless elsewhere.
2019-02-13 13:27:20 +01:00
Aaron Gyes
8d9089c78b Revert "io.cpp: use BUFFER_SIZE"
This reverts commit c931e33759.
2019-02-12 15:50:43 -08:00
Aaron Gyes
c931e33759 io.cpp: use BUFFER_SIZE
-Wunused-macros showed that a recent change used 4096 instead of
BUFFER_SIZE as previously (also 4096).
2019-02-12 15:37:09 -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
dbe906b79e Introduce dup2_list_t
This represents a "resolved" io_chain_t, where all of the different io_data_t
types have been reduced to a sequence of dup2() and close(). This will
eliminate a lot of the logic duplication around posix_spawn vs fork, and pave
the way for in-process redirections.
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