Commit Graph

58 Commits

Author SHA1 Message Date
ridiculousfish
2a4c545b21 Rework how signals trigger cancellation
When fish receives a "cancellation inducing" signal (SIGINT in particular)
it has to unwind execution - for example while loops or whatever else that
is executing. There are two ways this may come about:

1. The fish process received the signal
2. A child process received the signal

An example of the second case is:

    some_command | some_function

Here `some_command` is the tty owner and so will receive control-C, but
then fish has to cancel function execution.

Prior to this change, these were handled uniformly: both would just set a
cancellation signal inside the parser. However in the future we will have
multiple parsers and it may not be obvious which one to set the flag in.
So instead distinguish these cases: if a process receives SIGINT we mark
the signal in its job group, and if fish receives it we set a global
variable.
2020-07-12 12:16:01 -07:00
Mahmoud Al-Qudsi
04c6442dcc Allow fish_exit to run even on fish SIGHUP
We were previously aborting the main event loop before calling fish_exit
in the event of a SIGHUP. This patch causes the SIGHUP to be stored in a
separate state variable from a regular "must exit" condition so the
associated event can be fired before we terminate the loop.

All streams are redirected before the event is called to prevent a
SIGTTIN/SIGTTOU due to the user script reading/writing from a disposed
tty.

Closes #7014
2020-07-05 22:18:21 -05:00
Mahmoud Al-Qudsi
f2d5f95396 Merge sigint_checker_t generalizations from #7060
This makes it possible to expand the signals checked by the type. I can't merge
the sigttin fixes for #7060 yet because they introduce new breakage, but this
will make merging any future fix easier.
2020-06-20 11:27:15 -05:00
ridiculousfish
c7160d7cb4 Eliminate the termsize handling from common.h
Finish the transition to termsize.h. Remove the scary termsize bits
from common.cpp, which can throw off events at arbitrary calls and are
dangerously reentrant. Migrate everyone to the new termsize.h.
2020-06-07 20:00:42 -07:00
ridiculousfish
340c8490f6 Introduce termsize_container_t
fish's handling of terminal sizes is currently rather twisted. The
essential problem is that the terminal size may change at any point from a
SIGWINCH, and common_get_{width,height} may modify it and post variable
change events from arbitrary locations.

Tighten up the semantics. Assign responsibility for managing the tty size
to a new class, `termsize_container_t`. Rationalize locking and reentrancy.

Explicitly nail down the relationship between $COLUMNS/$LINES and the tty
size. The new semantics are: whatever changed most recently takes
precendence.
2020-06-07 20:00:42 -07:00
ridiculousfish
3ae91f197d Remove SIGTTOU handler before restoring foreground process group
When fish exits, it tries to restore the foreground process group.
However this may actually steal control of the fg process group
from another process. Fix this by clearing the SIGTTOU handler so
that tcsetpgrp() will fail.

Credit to @mqudsi for awesome debugging.

Fixes #7060
2020-05-31 14:11:39 -07:00
Rosen Penev
473a5250ae [clang-tidy] change several member functions to const
Found with readability-make-member-function-const

Signed-off-by: Rosen Penev <rosenp@gmail.com>
2020-04-05 10:13:13 +02: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
4a2c709fb1 Eliminate shell_is_interactive
We used to have a global notion of "is the shell interactive" but soon we
will want to have multiple independent execution threads, only some of
which may be interactive. Start tracking this data per-parser.
2019-06-29 11:28:26 -07:00
ridiculousfish
e3f94f4b72 Centralize signal handling into a single function
Prior to this diff, fish had different signal handling functions for
different signals. However it was hard to coordinate when a signal needed
to be the default handler, and when it was custom. In #5962 we overwrote
fish's custom WINCH handler with the default_handler when fish script asked
for WINCH to be handled.

Just have a single big signal handler function. That way it can never be
set to the wrong thing.

Fixes #5969
2019-06-28 10:51:22 -07:00
ridiculousfish
8254342339 Remove the useless second parameter from signal_handle
It was always set to 1.
2019-06-28 10:33:03 -07:00
Fabian Homborg
caedf01c00 Revert "Latch signal handlers"
This reverts commit 7ed1022cf4.

Fixes #5962.
2019-06-25 11:25:09 +02:00
ridiculousfish
7ed1022cf4 Latch signal handlers
Now that our interactive signal handlers are a strict superset of
non-interactive ones, there is no reason to "reset" signals or take action
when becoming non-interactive. Clean up how signal handlers get installed.
2019-05-26 18:04:03 -07:00
ridiculousfish
ead7f28026 Unify the SIGQUIT handlers and remove set_non_interactive_handlers
Interactive signal handlers are now a strict superset of non-interactive
ones. Remove set_non_interactive_handlers()
2019-05-26 13:18:06 -07:00
ridiculousfish
d729d57d94 Unify the interactive and non-interactive signal handlers
The signal handlers for interactive and non-interactive SIGINT were distinct
and talked to the reader. This wasn't really justified and will complicate
having multiple threads. Unify these into a single signal handler.
2019-05-26 00:32:11 -07:00
ridiculousfish
5a4df9dd3a Use sigint_checker_t in debug_thread_error 2019-05-25 19:19:03 -07:00
ridiculousfish
ea3ad0c099 Introduce sigint_checker_t and use it in wait
Allow a simpler way to check for sigint via sigint_checker_t.
Adopt it in builtin_wait, instead of hooking into the reader.
2019-05-25 19:08:04 -07:00
ridiculousfish
363652ad76 debug_thread_error to wait for a signal instead of sleep
Allows for control-C out of it
2019-05-21 20:15:59 -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
ridiculousfish
4b292c777d Clean up and clarify reader_exit() 2019-02-24 13:24:03 -08:00
ridiculousfish
130f2266d0 Remove the last of the signal blocking and checks
fish's signal handlers are now sufficiently innocuous that there should
be no reason to block signals (outside of temporarily, when creating a
thread and we need to manipulate the signal mask).
2019-02-23 14:07:35 -08:00
ridiculousfish
1b8ddacfed Reimplement signal handling event machinery
Prior to this fix, fish had a signal_list_t that accumulated signals.
Signals were added to an array of integers, with an overflow flag.
The event machinery would attempt to atomically "swap in" the other list.

After this fix, there is a single list of pending signal events, as an array
of atomic booleans. The signal handler sets the boolean corresponding to its
signal.
2019-02-23 13:03:33 -08:00
ridiculousfish
6bddf2c83b Add basic signal test 2019-02-23 11:57:19 -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
ridiculousfish
a4dc04a28e Add sighupint topic
This corresponds to SIGHUP and SIGINT. This will be used to break out of
process_mark_finished_children().
2019-02-17 13:01:59 -08:00
ridiculousfish
fc9d238642 Introduce topic monitoring
topic_monitor allows for querying changes posted to one or more topics,
initially sigchld. This will eventually replace the waitpid logic in
process_mark_finished_children().

Comment from the new header:

Topic monitoring support. Topics are conceptually "a thing that can
happen." For example, delivery of a SIGINT, a child process exits, etc. It
is possible to post to a topic, which means that that thing happened.

Associated with each topic is a current generation, which is a 64 bit
value. When you query a topic, you get back a generation. If on the next
query the generation has increased, then it indicates someone posted to
the topic.

For example, if you are monitoring a child process, you can query the
sigchld topic. If it has increased since your last query, it is possible
that your child process has exited.

Topic postings may be coalesced. That is there may be two posts to a given
topic, yet the generation only increases by 1. The only guarantee is that
after a topic post, the current generation value is larger than any value
previously queried.

Tying this all together is the topic_monitor_t. This provides the current
topic generations, and also provides the ability to perform a blocking
wait for any topic to change in a particular topic set. This is the real
power of topics: you can wait for a sigchld signal OR a thread exit.
2019-02-17 13:01:59 -08:00
ridiculousfish
83884c2c13 Improve reliability of signal handling in child processes
fish forks child processes when (for example) writing out builtin output.
After fork it resets signal handlers, but if a signal is delivered before
the signal handlers are reset, it will inherit fish's default handlers,
which do things like swallow SIGINT. Teach fish's default signal handlers
to detect this case and re-raise signals with default handlers.

This improves the reliability of control-C in the face of builtins.
2019-02-10 12:57:54 -08:00
ridiculousfish
28e2cfeb4b Switch signal table lookups to range-based for loops 2019-02-10 12:07:48 -08:00
ridiculousfish
f212518d3e Allow SIGINT in non-interactive mode
Prior to this fix, fish would swallow SIGINT in non-interactive mode. This
meant that scripts could only be Ctrl-C'd if fish was executing an external
command.

Unblock SIGINT in non-interactive mode.

Fixes #5253
2018-10-12 23:58:14 -07:00
Mahmoud Al-Qudsi
e656654456 Fix uninitialized sigaction.sa_flags valgrind error
Valgrind warns that the sometimes uninitialized sigaction.sa_flags field
is sometimes used when passed to the signal handler.

This patch explicitly zeros out the sigaction.sa_flags field at creation
time.
2017-08-26 19:13:58 -05: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
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
Kurtis Rader
35ee28ff24 Revert "finish cleanup of signal blocking code"
This reverts commit fb08fe5f47.

Needed to cleanly apply PR#4268. Will reapply after applying that
change.
2017-08-06 14:38:25 -07:00
Kurtis Rader
fb08fe5f47 finish cleanup of signal blocking code
PR #3691 made most calls to `signal_block()` and `signal_unblock()`
no-ops unless a magic env var is set when fish starts running. It's
been seven months since that change was made and no problems have been
reported. This finishes that work by removing those no-op function calls
and support for the magic env var in our next major release (which won't
happen till at least six months from now).
2017-07-26 13:51:00 -07:00
Kurtis Rader
fddd5fd045 fix how SIGHUP is handled when ignored
Working on a related problem caused me to notice that if a fish script
was run via `nohup` it would die when receiving SIGHUP. This fixes the
code to handle that correctly so that fish scripts can be nohup'd.

Fixes #4007
2017-05-03 22:16:41 -07:00
Kurtis Rader
107127afb7 ensure no signals are blocked when started
Fixes #3964
2017-05-02 21:02:42 -07:00
Kurtis Rader
e0f62c178f make not blocking signals the default
This is the next step in determining whether we can disable blocking
signals without a good reason to do so. This makes not blocking signals
the default behavior. If someone finds a problem they can add this to
their ~/config/fish/config.fish file:

set FISH_NO_SIGNAL_BLOCK 0

Alternatively set that env var before starting fish. I won't be surprised
if people report problems. Till now we have relied on people opting in
to this behavior to tell us whether it causes problems. This makes the
experimental behavior the default that has to be opted out of. This will
give us a lot more confidence this change doesn't cause problems before
the next minor release.

Note that there are still a few places where we force blocking of
signals. Primarily to keep SIGTSTP from interfering with the shell in
response to manipulating the controlling tty. Bash is more selective
in the signals it blocks around the problematic syscalls (c.f., its
`git_terminal_to()` function). However, I don't see any value in that
refinement.
2017-03-10 21:34:24 -08:00
Kurtis Rader
6d02bec4c7 fix setting up and using the terminfo data
There should be just one place that calls `setupterm()`. While refactoring
the code I also decided to not make initializing the curses subsystem a
fatal error. We now try two fallback terminal names ("ansi" and "dumb")
and if those can't be used we still end up with a usable shell.

Fixes #3850
2017-03-08 20:54:37 -08:00
Kurtis Rader
ea9e05568e Revert "make not blocking signals the default"
This reverts commit e30f3fee88.

Not sure why I didn't notice this before merging it but the change I'm
reverting makes it impossible to start a login shell.
2017-02-25 21:06:30 -08:00
Kurtis Rader
e30f3fee88 make not blocking signals the default
This is the next step in determining whether we can disable blocking
signals without a good reason to do so. This makes not blocking signals
the default behavior. If someone finds a problem they can add this to
their ~/config/fish/config.fish file:

set FISH_NO_SIGNAL_BLOCK 0

Alternatively set that env var before starting fish. I won't be surprised
if people report problems. Till now we have relied on people opting in
to this behavior to tell us whether it causes problems. This makes the
experimental behavior the default that has to be opted out of. This will
give is a lot more confidence this change doesn't cause major problems
prior to the next minor release.
2017-02-22 21:53:49 -08:00
Kurtis Rader
4ad5b756e4 more sanity involving fatal errors
This folds the "VOMIT_*" family of macros into the assert and DIE
family.

Another change related to issue #3276.
2017-02-14 21:09:15 -08:00
ridiculousfish
d8a6c0a91b Add a SIGALRM handler that does nothing
This will be part of a future signal torture-test, to ensure
we are handling EINTR correctly
2017-01-21 14:07:54 -08:00
Kurtis Rader
fd6d814ea4 remove unnecessary signal management
The shell was doing a log of signal blocking/unblocking that hurts
performance and can be avoided. This reduced the elapsed time for a
simple benchmark by 25%.

Partial fix for #2007
2017-01-14 20:51:54 -08:00
Kurtis Rader
a928517e95 improve converting strings to ints/longs
The existing code is inconsistent, and in a couple of cases wrong, about
dealing with strings that are not valid ints. For example, there are
locations that call wcstol() and check errno without first setting errno
to zero. Normalize the code to a consistent pattern.  This is mostly to
deal with inconsistencies between BSD, GNU, and other UNIXes.

This does make some syntax more liberal. For example `echo $PATH[1 .. 3]`
is now valid due to uniformly allowing leading and trailing whitespace
around numbers. Whereas prior to this change you would get a "Invalid
index value" error. Contrast this with `echo $PATH[ 1.. 3 ]` which was
valid and still is.
2016-11-25 18:52:15 -08:00
Kurtis Rader
521546a986 fix some lint warnings
This fixes some of the IWYU and cppcheck lint warnings. And only on
macOS (formerly OS X). Fixing these types of warnings on a broader set
of platforms should be done but this is a baby step to making `make
lint-all` have few, if any, warnings. This reduces the number of lines
in the `make lint-all` output on macOS by over 500 lines.
2016-11-15 21:15:22 -08:00
Kurtis Rader
f382fa8e8a lint: multiple unary operator 2016-10-27 21:21:54 -07:00
Kurtis Rader
c07c98ac05 eliminate many "unused parameter" warnings
Partially addresses issue #3430.
2016-10-09 14:38:26 -07:00
ridiculousfish
d17f2585fa Revert "Tighten up empty string checks."
The use of wcstoimax causes certain out-of-range values
to be silently truncated (e.g. when converted to a pid),
and is incompatible with FreeBSD (see #626)

This reverts commit 6faa2f9866.
2016-10-01 17:21:40 -07:00
Aaron Gyes
6faa2f9866 Tighten up empty string checks.
Fixes various spots throughout fish where broken strtoi checks
were converting empty strings to zero. Zero is not a valid pid and
this was causing breakage as well when input.

Nix fish_wcstoi - wcstoimax does the same thing.
Improve comments and some general cleanup.
2016-09-10 14:46:06 -07:00
Aaron Gyes
fa78a7101c Make IWYU output in lint.cpp less messy
And re-run IWYU, adjust #includes.
2016-06-23 17:26:08 -07:00