Mirrors/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell synced 2024-11-14 17:07:44 +00:00
Code Issues Projects Releases Packages Wiki Activity

Revert "Revert "finish cleanup of signal blocking code""

Browse source 
This reverts commit 35ee28ff24.
It was meant for the major branch.
This commit is contained in:
Kurtis Rader 2017-08-13 15:30:34 -07:00
parent 7776a03fb3
commit 7dd2cd7de7
8 changed files with 18 additions and 86 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
src/exec.cpp
View file

@ -36,7 +36,6 @@
#include "postfork.h"
#include "proc.h"
#include "reader.h"
#include "signal.h"
#include "wutil.h" // IWYU pragma: keep
/// File descriptor redirection error message.
@ -322,16 +321,12 @@ static void internal_exec_helper(parser_t &parser, const wcstring &def, node_off
return;
}
signal_unblock();
if (node_offset == NODE_OFFSET_INVALID) {
parser.eval(def, morphed_chain, block_type);
} else {
parser.eval_block_node(node_offset, morphed_chain, block_type);
}
signal_block();
morphed_chain.clear();
io_cleanup_fds(opened_fds);
job_reap(0);
@ -401,10 +396,8 @@ void exec_job(parser_t &parser, job_t *j) {
if (j->processes.front()->type == INTERNAL_EXEC) {
// Do a regular launch - but without forking first...
signal_block();
// setup_child_process makes sure signals are properly set up. It will also call
// signal_unblock.
// setup_child_process makes sure signals are properly set up.
// PCA This is for handling exec. Passing all_ios here matches what fish 2.0.0 and 1.x did.
// It's known to be wrong - for example, it means that redirections bound for subsequent
@ -448,8 +441,6 @@ void exec_job(parser_t &parser, job_t *j) {
}
}
signal_block();
// See if we need to create a group keepalive process. This is a process that we create to make
// sure that the process group doesn't die accidentally, and is often needed when a
// builtin/block/function is inside a pipeline, since that usually means we have to wait for one
@ -620,9 +611,6 @@ void exec_job(parser_t &parser, job_t *j) {
switch (p->type) {
case INTERNAL_FUNCTION: {
// Calls to function_get_definition might need to source a file as a part of
// autoloading, hence there must be no blocks.
signal_unblock();
const wcstring func_name = p->argv0();
wcstring def;
bool function_exists = function_get_definition(func_name, &def);
@ -630,8 +618,6 @@ void exec_job(parser_t &parser, job_t *j) {
const std::map<wcstring, env_var_t> inherit_vars =
function_get_inherit_vars(func_name);
signal_block();
if (!function_exists) {
debug(0, _(L"Unknown function '%ls'"), p->argv0());
break;
@ -639,13 +625,7 @@ void exec_job(parser_t &parser, job_t *j) {
function_block_t *fb =
parser.push_block<function_block_t>(p, func_name, shadow_scope);
// Setting variables might trigger an event handler, hence we need to unblock
// signals.
signal_unblock();
function_prepare_environment(func_name, p->get_argv() + 1, inherit_vars);
signal_block();
parser.forbid_function(func_name);
if (!p->is_last_in_job) {
@ -795,12 +775,8 @@ void exec_job(parser_t &parser, job_t *j) {
const int fg = j->get_flag(JOB_FOREGROUND);
j->set_flag(JOB_FOREGROUND, false);
signal_unblock();
p->status = builtin_run(parser, p->get_argv(), *builtin_io_streams);
signal_block();
// Restore the fg flag, which is temporarily set to false during builtin
// execution so as not to confuse some job-handling builtins.
j->set_flag(JOB_FOREGROUND, fg);
@ -1113,9 +1089,7 @@ void exec_job(parser_t &parser, job_t *j) {
kill(keepalive.pid, SIGKILL);
}
signal_unblock();
debug(3, L"Job is constructed");
j->set_flag(JOB_CONSTRUCTED, true);
if (!j->get_flag(JOB_FOREGROUND)) {
proc_last_bg_pid = j->pgid;

7
src/fish.cpp
View file

@ -387,13 +387,6 @@ int main(int argc, char **argv) {
const io_chain_t empty_ios;
if (read_init(paths)) {
// TODO: Remove this once we're confident that not blocking/unblocking every signal around
// some critical sections is no longer necessary.
env_var_t fish_no_signal_block = env_get_string(L"FISH_NO_SIGNAL_BLOCK");
if (!fish_no_signal_block.missing_or_empty() && !from_string<bool>(fish_no_signal_block)) {
ignore_signal_block = false;
}
// Stomp the exit status of any initialization commands (issue #635).
proc_set_last_status(STATUS_CMD_OK);

9
src/history.cpp
View file

@ -39,7 +39,6 @@
#include "parse_util.h"
#include "path.h"
#include "reader.h"
#include "signal.h"
#include "wutil.h" // IWYU pragma: keep
// Our history format is intended to be valid YAML. Here it is:
@ -998,9 +997,6 @@ bool history_t::load_old_if_needed(void) {
if (loaded_old) return true;
loaded_old = true;
// PCA not sure why signals were blocked here
// signal_block();
bool ok = false;
if (map_file(name, &mmap_start, &mmap_length, &mmap_file_id)) {
// Here we've mapped the file.
@ -1009,7 +1005,6 @@ bool history_t::load_old_if_needed(void) {
this->populate_from_mmap();
}
// signal_unblock();
return ok;
}
@ -1409,8 +1404,6 @@ bool history_t::save_internal_via_appending() {
return true;
}
signal_block();
// We are going to open the file, lock it, append to it, and then close it
// After locking it, we need to stat the file at the path; if there is a new file there, it
// means
@ -1498,8 +1491,6 @@ bool history_t::save_internal_via_appending() {
close(history_fd);
}
signal_unblock();
// If someone has replaced the file, forget our file state.
if (file_changed) {
this->clear_file_state();

6
src/postfork.cpp
View file

@ -109,9 +109,9 @@ bool set_child_group(job_t *j, process_t *p, int print_errors) {
int result = -1;
errno = EINTR;
while (result == -1 && errno == EINTR) {
signal_block(true);
signal_block();
result = tcsetpgrp(STDIN_FILENO, j->pgid);
signal_unblock(true);
signal_unblock();
}
if (result == -1) {
if (errno == ENOTTY) redirect_tty_output();
@ -251,8 +251,6 @@ int setup_child_process(job_t *j, process_t *p, const io_chain_t &io_chain) {
signal_reset_handlers();
}
signal_unblock(); // remove all signal blocks
return ok ? 0 : -1;
}

16
src/proc.cpp
View file

@ -789,7 +789,7 @@ static bool terminal_give_to_job(job_t *j, int cont) {
return true;
}
signal_block(true);
signal_block();
int result = -1;
errno = EINTR;
while (result == -1 && errno == EINTR) {
@ -799,7 +799,7 @@ static bool terminal_give_to_job(job_t *j, int cont) {
if (errno == ENOTTY) redirect_tty_output();
debug(1, _(L"Could not send job %d ('%ls') to foreground"), j->job_id, j->command_wcstr());
wperror(L"tcsetpgrp");
signal_unblock(true);
signal_unblock();
return false;
}
@ -817,12 +817,12 @@ static bool terminal_give_to_job(job_t *j, int cont) {
debug(1, _(L"terminal_give_to_job(): Could not send job %d ('%ls') to foreground"),
j->job_id, j->command_wcstr());
wperror(L"tcsetattr");
signal_unblock(true);
signal_unblock();
return false;
}
}
signal_unblock(true);
signal_unblock();
return true;
}
@ -835,12 +835,12 @@ static bool terminal_return_from_job(job_t *j) {
return true;
}
signal_block(true);
signal_block();
if (tcsetpgrp(STDIN_FILENO, getpgrp()) == -1) {
if (errno == ENOTTY) redirect_tty_output();
debug(1, _(L"Could not return shell to foreground"));
wperror(L"tcsetpgrp");
signal_unblock(true);
signal_unblock();
return false;
}
@ -849,7 +849,7 @@ static bool terminal_return_from_job(job_t *j) {
if (errno == EIO) redirect_tty_output();
debug(1, _(L"Could not return shell to foreground"));
wperror(L"tcgetattr");
signal_unblock(true);
signal_unblock();
return false;
}
@ -867,7 +867,7 @@ static bool terminal_return_from_job(job_t *j) {
}
#endif
signal_unblock(true);
signal_unblock();
return true;
}

16
src/reader.cpp
View file

@ -1575,20 +1575,12 @@ static void reader_interactive_init() {
// Check if we are in control of the terminal, so that we don't do semi-expensive things like
// reset signal handlers unless we really have to, which we often don't.
if (tcgetpgrp(STDIN_FILENO) != shell_pgid) {
int block_count = 0;
int i;
// Bummer, we are not in control of the terminal. Stop until parent has given us control of
// it. Stopping in fish is a bit of a challange, what with all the signal fidgeting, we need
// to reset a bunch of signal state, making this coda a but unobvious.
// it.
//
// In theory, reseting signal handlers could cause us to miss signal deliveries. In
// practice, this code should only be run suring startup, when we're not waiting for any
// practice, this code should only be run during startup, when we're not waiting for any
// signals.
while (signal_is_blocked()) {
signal_unblock();
block_count++;
}
signal_reset_handlers();
// Ok, signal handlers are taken out of the picture. Stop ourself in a loop until we are in
@ -1632,10 +1624,6 @@ static void reader_interactive_init() {
}
signal_set_handlers();
for (i = 0; i < block_count; i++) {
signal_block();
}
}
// Put ourselves in our own process group.

16
src/signal.cpp
View file

@ -16,9 +16,6 @@
#include "reader.h"
#include "wutil.h" // IWYU pragma: keep
// This is a temporary var while we explore whether signal_block() and friends is needed.
bool ignore_signal_block = true;
/// Struct describing an entry for the lookup table used to convert between signal names and signal
/// ids, etc.
struct lookup_entry {
@ -383,9 +380,7 @@ void get_signals_with_handlers(sigset_t *set) {
}
}
void signal_block(bool force) {
if (!force && ignore_signal_block) return;
void signal_block() {
ASSERT_IS_MAIN_THREAD();
sigset_t chldset;
@ -405,14 +400,10 @@ void signal_unblock_all() {
sigprocmask(SIG_SETMASK, &iset, NULL);
}
void signal_unblock(bool force) {
if (!force && ignore_signal_block) return;
void signal_unblock() {
ASSERT_IS_MAIN_THREAD();
sigset_t chldset;
block_count--;
if (block_count < 0) {
debug(0, _(L"Signal block mismatch"));
bugreport();
@ -420,13 +411,12 @@ void signal_unblock(bool force) {
}
if (!block_count) {
sigset_t chldset;
sigfillset(&chldset);
DIE_ON_FAILURE(pthread_sigmask(SIG_UNBLOCK, &chldset, 0));
}
// debug( 0, L"signal block level decreased to %d", block_count );
}
bool signal_is_blocked() {
if (ignore_signal_block) return false;
return static_cast<bool>(block_count);
}

6
src/signal.h
View file

@ -30,16 +30,14 @@ void signal_handle(int sig, int do_handle);
void signal_unblock_all();
/// Block all signals.
void signal_block(bool force = false);
void signal_block();
/// Unblock all signals.
void signal_unblock(bool force = false);
void signal_unblock();
/// Returns true if signals are being blocked.
bool signal_is_blocked();
/// Returns signals with non-default handlers.
void get_signals_with_handlers(sigset_t *set);
extern bool ignore_signal_block;
#endif