Commit Graph

14575 Commits

Author SHA1 Message Date
ridiculousfish
4b9a096cf2 builtins to sometimes not buffer when writing to a pipe
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.
2021-02-08 14:22:02 -08:00
ridiculousfish
171d09288b Rename allow_buffering to piped_output_needs_buffering
This makes the variable's role clear. It controls whether output to a
pipe must be buffered to avoid deadlock.
2021-02-08 14:22:02 -08:00
Mahmoud Al-Qudsi
2d39568ec4 Statically assert the sort order of more lists
Add compile-time checks to ensure list of string subcommands, builtins,
and electric variables are kept in asciibetical order to facilitate
binary search lookups.
2021-02-08 15:31:49 -06:00
Mahmoud Al-Qudsi
cb3ab80cab Use const_strlen in a few different places
This may slightly improve performance by allowing the compiler greater
visibility into what is happing on top of not executing at runtime in
some hot paths, but more importantly, it gets rid of magic constants in a
few different places.
2021-02-08 15:16:21 -06:00
Mahmoud Al-Qudsi
5d4c24bae1 Refactor color.h/color.cpp
* Use `uint8_t` instead of `unsigned char`
* Statically assert the sort order for `named_colors`
* Use constexpr for array lengths
2021-02-08 15:16:21 -06:00
Mahmoud Al-Qudsi
5fda1e05dc Statically assert the sort order of input_function_metadata_t 2021-02-08 15:16:20 -06:00
Mahmoud Al-Qudsi
2df16b53ff Use thread-local vectors for caching peeked events
These functions are called in the event queue hot path every time an
input event takes place. If we could guarantee a maximum length of
non-char (i.e. readline) events in the queue, we could use
`event_queue_peeker_t` with a fixed storage size of, e.g., 32 events,
but I'm not sure what a reasonable number would in fact be, so I'm just
changing these to use a thread-local vector that will re-use its
previous heap allocation in subsequent invocations rather than thrashing
the heap.
2021-02-08 15:16:20 -06:00
Mahmoud Al-Qudsi
2c1764fd45 Convert more event queue push_front loops to insert_front 2021-02-08 15:16:20 -06:00
Mahmoud Al-Qudsi
5c014e129a Reduce input latency searching for readline function mappings
The lookups are executed on all input events, so they are worth
optimizing.

Cache the list of names, use binary search to get a function code from a
name, and stop enumerating mappings after `has_function` and `has_command`
have been determined.
2021-02-08 15:16:20 -06:00
Ethel Morgan
6dd6a57c60 Saturate return value in builtin_set_query
builtin_set_query returns the number of missing variables. Because the
return value passed to the shell is an 8-bit unsigned integer, if the
number of missing variables is a multiple of 256, it would overflow to 0.

This commit saturates the return value at 255 if there are more than 255
missing variables.
2021-02-08 20:38:56 +01:00
Fabian Homborg
42c75111c8 CHANGELOG: Fix code inside bold blocks
Sphinx doesn't like it, so either use normal quotes or put the code
outside of the bold block.
2021-02-08 17:18:53 +01:00
Michael Jarvis
84a89f5195 Fix sphinx warning
[100%] Building HTML documentation with Sphinx
../CHANGELOG.rst:48: WARNING: Document or section may not begin with a transition.
../CHANGELOG.rst:48: WARNING: Document or section may not begin with a transition.
2021-02-08 17:16:40 +01:00
Johannes Altmanninger
e487f193b8 CHANGELOG: add headers for fish-next-minor 2021-02-08 07:31:33 +01:00
Johannes Altmanninger
330883b6fd Run fish_indent on share/**.fish
This is mostly to show that some of my following indent changes don't break
current behavior.
2021-02-08 07:31:33 +01:00
Johannes Altmanninger
e40850ab89 Format fish_tests.cpp 2021-02-08 07:31:33 +01:00
ridiculousfish
a082cf138a Add a Dockerfile to test 32 bit builds
Run it with:

    ./docker/docker_run_tests.sh docker/focal-32bit.Dockerfile
2021-02-07 17:51:31 -08:00
ridiculousfish
50a7798041 Elimiate static variables inside builtin_test
builtin_test stashes some variables in statics, to support
the `test -t` expression. However this will cause conflicts with
concurrent execution, where we may want to run two `test` expressions at
once. Do the grunt work of threading the data into all places it needs
to go.
2021-02-07 17:41:36 -08:00
ridiculousfish
40d8e7e983 Correct the sense of a test for builtin stdin fds
fish isn't quite sure what to do if the user specifies an fd redirection
for builtins. For example `source <&5` could potentially just read from
an arbitrary file descriptor internal to fish, like the history file.

fish has some lame code that tries to detect these, but got the sense
wrong. Fix it so that fd redirections for builtins are restricted to
range 0 through 2.
2021-02-07 16:21:33 -08:00
ridiculousfish
17707065b8 Remove the io_pipe_t parameter from exec_internal_builtin_proc
This parameter describes if stdin has a pipe, but that can be easily
inferred from the io_chain. Remove it in the interest of parsimony.
2021-02-07 16:03:58 -08:00
Fabian Homborg
48b3b826aa CHANGELOG fish_color_keyword 2021-02-07 21:20:34 +01:00
Fabian Homborg
b3626d48e7 Highlight keywords differently
This introduces a new variable $fish_color_keyword that will be used
to highlight keywords. If it's not defined, we fall back on
$fish_color_command as before.

An issue here is that most of our keywords have this weird duality of
also being builtins *if* executed without an argument or with
`--help`.

This means that e.g.

    if

is highlighted as a command until you start typing

    if t

and then it turns keyword.
2021-02-07 21:18:51 +01:00
ridiculousfish
96f2de9d15 Revert "Disable Github Actions tsan"
This reverts commit 432f005859.

Thread Sanitizer issues have been sorted, so bravely re-enable
this test.
2021-02-07 10:59:10 -08:00
ridiculousfish
0f1281bec6 Unify thread sanitizer detection
We now have two files that need to know if thread sanitizer is enabled. They
can share the detection code.
2021-02-07 10:59:10 -08:00
ridiculousfish
ced56d492f Disable iothread pool wait-around under TSan
The iothread pool has a feature where, if the thread is emptied, some
threads will choose to wait around in case new work appears, up to a
certain amount of time (500 msec). This prevents thrashing where new
threads are rapidly created and destroyed as the user types. This is
implemented via `std::condition_variable::wait_for`. However this function
is not properly instrumented under Thread Sanitizer (see
https://github.com/google/sanitizers/issues/1259) so TSan reports false
positives. Just disable this feature under TSan.
2021-02-07 10:59:10 -08:00
ridiculousfish
a942df3886 Use fd_event_signaller_t in fd_monitor_t
fd_monitor_t allows observing a collection of fds. It also has its own
fd, which it uses to awaken itself when there are changes. Switch to
using fd_event_signaller_t instead of a pipe; this reduces the number of
file descriptors and is more efficient under Linux.
2021-02-07 10:59:10 -08:00
ridiculousfish
e004930947 Use fd_event_signaller in iothread completions
This simplifies how iothread notices when there are completions ready to
run.
2021-02-07 10:59:10 -08:00
ridiculousfish
8066428feb Add fd_event_signaller_t
fd_event_signaller_t exists to expose eventfd under Linux. This is a
more lightweight way of signalling events than using a pipe.
2021-02-07 10:59:10 -08:00
Fabian Homborg
5152838417 tests: Move fg2.py into fg.py
There's no reason for this to be a separate file.
2021-02-07 19:52:32 +01:00
Fabian Homborg
78358ab351 Make disable_mouse_tracking inaccessible
This isn't something you want to bind, it's only a readline symbol as
a hack, so we shouldn't expose it to the user.
2021-02-07 19:50:56 +01:00
Mahmoud Al-Qudsi
1651db23fe Update CHANGELOG to add issue 4873 to 3.2.0 release 2021-02-07 10:36:38 -06:00
David Adam
c633ce7e76 CHANGELOG: work on 3.2.0 2021-02-07 22:14:03 +08:00
Ilya Grigoriev
254729f86d Fix Alt+L when using the "Informative" prompt
I ran into problems described in https://github.com/fish-shell/fish-shell/issues/718 when using this prompt. This seems to be a bug in the prompt -- this change fixes it, at least on my system. 

I tried this in tmux (TERM=screen) and gnome-terminal (TERM=xterm-256) with fish 3.1.2, on Linux.
2021-02-07 08:54:02 +01:00
ridiculousfish
aac5862a67 Use vectors, not queues, in iothread main thread requests
queues use std::deque under the hood which is more expensive than a vector.
We always consume the entire queue so there is no advantage to use deque here.
Just use a vector.
2021-02-06 16:19:21 -08:00
ridiculousfish
76833cf6af Use futures in perform_on_main_thread
Replace the complicated implementation which shared a condition variable, with
one which just uses std::future<void>. This may allocate more condition
variables but is much simpler.
2021-02-06 16:19:21 -08:00
Mahmoud Al-Qudsi
ae1c53cc19 Merge branch 'disable_mouse_tracking' 2021-02-06 17:25:36 -06:00
Mahmoud Al-Qudsi
eecc223c51 Recognize and disable mouse-tracking CSI events
Fish was previously oblivious to the existence of mouse-tracking ANSI
escapes; this was mostly OK because they're disabled by default and we
don't enable them, but if a TUI application that turned on mouse
reporting crashed or exited without turning mouse reporting off, fish
would be left in an unusable state as all mouse reporting CSI sequences
would be posted to the prompt.

This can be tested by executing `printf '\x1b[?1003h'` at the prompt,
then clicking with any mouse button anywhere within the terminal window.
Previously, this would have resulted in seeming garbage being spewed to
the prompt; now, fish detects the mouse tracking CSIs posted to stdin by
the terminal emulator and a) ignores them to prevent invalid input, as
well as b) posts the CSI needed to disable future mouse tracking events
from being emitted on subsequent mouse interactions (until re-enabled).

Note that since we respond to a mouse tracking CSI rather than
pre-emptively disable mouse reporting, we do not need to do any sort of
feature detection to determine whether or not the terminal supports
mouse reporting (otherwise, if it didn't support it and we posted the
CSI anyway, we'd end up with exactly the kind of cruft posted to the
prompt that we're trying to avoid).

Fixes #4873
2021-02-06 17:22:59 -06:00
Mahmoud Al-Qudsi
cc392b3774 Add RAII-based event_queue_peeker_t helper
This is a stack-allocating utility class to peek up to N
characters/events out of an `event_queue_t` object. The need for a
hard-coded maximum peek length N at each call site is to avoid any heap
allocation, as this would be called in a hot path on every input event.
2021-02-06 17:18:53 -06:00
Mahmoud Al-Qudsi
c203c88c66 Add and use event_queue_t::insert_front()
This allows directly inserting multiple characters/events in one go at
the front of the input queue, instead of needing to add them one-by-one
in reverse order.

In addition to improving performance in case of fragmented dequeue
allocation, this also is less error prone since a dev need not remember
to use reverse iterators when looping over a vector of peeked events.
2021-02-06 17:18:53 -06:00
Mahmoud Al-Qudsi
b385772a84 fixup! Explicitly annotate intentional switch fallthrough
<manual git patch editing failure>
2021-02-06 17:17:30 -06:00
Mahmoud Al-Qudsi
ea1a4b7932 Explicitly annotate intentional switch fallthrough
This silences a very useful warning in GCC 10.
2021-02-06 17:03:23 -06:00
ridiculousfish
b7e892d545 next_thread_id to use atomics, not locks
We have multiple places where we use std::atomic<uint64_t>, so let's use it
in next_thread_id too.
2021-02-06 14:27:08 -08:00
ridiculousfish
fe334bf620 Remove scoped_rlock
It is unused.
2021-02-06 14:27:08 -08:00
ridiculousfish
9c238385f0 Fix binary_semaphore_t under non-Linux TSan
Under non-Linux builds, binary_semaphore is implemented with a
self-pipe. When TSan is active we mark the pipe as non-blocking as TSan
cannot interrupt read (but can interrupt select). However we weren't
properly testing for EAGAIN leading to an assertion failure.

Allow looping on EAGAIN.
2021-02-06 14:41:50 -08:00
ridiculousfish
736e344727 assert_is_locked to take std::mutex, not void *
It's unclear why this had the void* cast.
2021-02-06 14:24:45 -08:00
ridiculousfish
98b0ef532f io_buffer_t to store a promise, not a future, to satisfy TSan
io_buffer_t is a buffer that fills itself by reading from a file
descriptor (typically a pipe). When the file descriptor is widowed, the
operation completes, and it reports completion by marking a
`std::promise<void>`. The "main thread" waits for this by waiting on the
promise's future. However TSan was reporting that the future's destructor
races with its promise's wait method. It's not obvious if this is valid,
but we can fix it by keeping the promise alive until the io_buffer_t is
deallocated.

This fixes the TSan issues reported under
`complete_background_fillthread_and_take_buffer` for #7681 (but there
are other unresolved issues).
2021-02-06 13:28:01 -08:00
Fabian Homborg
432f005859 Disable Github Actions tsan
This was updated and now always fails, but it always did so - you can
test it with 3.1.2 as well, it's just not happy with the iothread
stuff.

Because it's super easy to test this locally this disables the github
actions test so it doesn't complain *constantly*.

See #7681
2021-02-06 21:32:42 +01:00
Fabian Homborg
caca4fec22 fds.h: Add missing types.h include
Broke the build on FreeBSD because that defines mode_t there.
2021-02-06 19:59:53 +01:00
ridiculousfish
b5716e97cc Remove fd_set_t
Now that we no longer need to worry about pipes conflicting with
user-specified redirections, we can remove fd_set_t.
2021-02-05 18:14:50 -08:00
ridiculousfish
b79ec0122a Use pipe2 when creating pipes if avaialble
This allows us to avoid marking the pipe as CLOEXEC in some cases,
saving a system call.
2021-02-05 17:58:08 -08:00
ridiculousfish
97f29b1f4d Pipe fds to move to the "high range"
This concerns how fish prevents its own fds from interfering with
user-defined fd redirections, like `echo hi >&5`. fish has historically
done this by tracking all user defined redirections when running a job,
and ensuring that pipes are not assigned the same fds. However this is
annoying to pass around - it means that we have to thread user-defined
redirections into pipe creation.

Take a page from zsh and just ensure that all pipes we create have fds in
the "high range," which here means at least 10. The primary way to do this
is via the F_DUPFD_CLOEXEC syscall, which also sets CLOEXEC, so we aren't
invoking additional syscalls in the common case. This will free us from
having to track which fds are in user-defined redirections.
2021-02-05 17:58:08 -08:00