From f1b208997c6c2b90e209a6335178cbaef8adb34b Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 23 Feb 2019 01:04:05 -0800 Subject: [PATCH] Cleanup events Prior to this fix, an "event" was used as both a predicate on which events to match, and also as the event itself. Re-express these concepts distinctly: an event is something that happened, an event_handler is the predicate and name of the function to execute. --- src/builtin_function.cpp | 17 +- src/builtin_functions.cpp | 22 +-- src/env.cpp | 27 +-- src/event.cpp | 388 ++++++++++++++------------------------ src/event.h | 99 +++++----- src/function.cpp | 8 +- src/function.h | 2 +- src/input.cpp | 2 +- src/proc.cpp | 10 +- 9 files changed, 216 insertions(+), 359 deletions(-) diff --git a/src/builtin_function.cpp b/src/builtin_function.cpp index bec67bff5..104a728d8 100644 --- a/src/builtin_function.cpp +++ b/src/builtin_function.cpp @@ -29,7 +29,7 @@ struct function_cmd_opts_t { bool print_help = false; bool shadow_scope = true; wcstring description = L""; - std::vector events; + std::vector events; wcstring_list_t named_arguments; wcstring_list_t inherit_vars; wcstring_list_t wrap_targets; @@ -68,7 +68,7 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig streams.err.append_format(_(L"%ls: Unknown signal '%ls'"), cmd, w.woptarg); return STATUS_INVALID_ARGS; } - opts.events.push_back(event_t::signal_event(sig)); + opts.events.push_back(event_description_t::signal(sig)); break; } case 'v': { @@ -77,17 +77,17 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig return STATUS_INVALID_ARGS; } - opts.events.push_back(event_t::variable_event(w.woptarg)); + opts.events.push_back(event_description_t::variable(w.woptarg)); break; } case 'e': { - opts.events.push_back(event_t::generic_event(w.woptarg)); + opts.events.push_back(event_description_t::generic(w.woptarg)); break; } case 'j': case 'p': { pid_t pid; - event_t e(event_type_t::any); + event_description_t e(event_type_t::any); if ((opt == 'j') && (wcscasecmp(w.woptarg, L"caller") == 0)) { job_id_t job_id = -1; @@ -251,16 +251,11 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis d.name = function_name; if (!opts.description.empty()) d.description = opts.description; // d.description = opts.description; - d.events.swap(opts.events); + d.events = std::move(opts.events); d.props.shadow_scope = opts.shadow_scope; d.props.named_arguments = std::move(opts.named_arguments); d.inherit_vars = std::move(opts.inherit_vars); - for (size_t i = 0; i < d.events.size(); i++) { - event_t &e = d.events.at(i); - e.function_name = d.name; - } - d.props.parsed_source = source; d.props.body_node = body; function_add(std::move(d), parser); diff --git a/src/builtin_functions.cpp b/src/builtin_functions.cpp index 1acb4e085..860f675e7 100644 --- a/src/builtin_functions.cpp +++ b/src/builtin_functions.cpp @@ -126,10 +126,7 @@ static wcstring functions_def(const wcstring &name) { wcstring desc, def; function_get_desc(name, desc); function_get_definition(name, def); - event_t search(event_type_t::any); - search.function_name = name; - std::vector> ev; - event_get(search, &ev); + std::vector> ev = event_get_function_handlers(name); out.append(L"function "); @@ -153,29 +150,30 @@ static wcstring functions_def(const wcstring &name) { } for (const auto &next : ev) { - switch (next->type) { + const event_description_t &d = next->desc; + switch (d.type) { case event_type_t::signal: { - append_format(out, L" --on-signal %ls", sig2wcs(next->param1.signal)); + append_format(out, L" --on-signal %ls", sig2wcs(d.param1.signal)); break; } case event_type_t::variable: { - append_format(out, L" --on-variable %ls", next->str_param1.c_str()); + append_format(out, L" --on-variable %ls", d.str_param1.c_str()); break; } case event_type_t::exit: { - if (next->param1.pid > 0) - append_format(out, L" --on-process-exit %d", next->param1.pid); + if (d.param1.pid > 0) + append_format(out, L" --on-process-exit %d", d.param1.pid); else - append_format(out, L" --on-job-exit %d", -next->param1.pid); + append_format(out, L" --on-job-exit %d", -d.param1.pid); break; } case event_type_t::job_exit: { - const job_t *j = job_t::from_job_id(next->param1.job_id); + const job_t *j = job_t::from_job_id(d.param1.job_id); if (j) append_format(out, L" --on-job-exit %d", j->pgid); break; } case event_type_t::generic: { - append_format(out, L" --on-event %ls", next->str_param1.c_str()); + append_format(out, L" --on-event %ls", d.str_param1.c_str()); break; } default: { diff --git a/src/env.cpp b/src/env.cpp index ae3725402..83cb2cf32 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -639,12 +639,7 @@ static void universal_callback(env_stack_t *stack, const callback_data_t &cb) { react_to_variable_change(op, cb.key, *stack); stack->mark_changed_exported(); - - event_t ev = event_t::variable_event(cb.key); - ev.arguments.push_back(L"VARIABLE"); - ev.arguments.push_back(op); - ev.arguments.push_back(cb.key); - event_fire(&ev); + event_fire(event_t::variable(cb.key, {L"VARIABLE", op, cb.key})); } /// Make sure the PATH variable contains something. @@ -1256,12 +1251,7 @@ int env_stack_t::set_internal(const wcstring &key, env_mode_flags_t input_var_mo } } - event_t ev = event_t::variable_event(key); - ev.arguments.reserve(3); - ev.arguments.push_back(L"VARIABLE"); - ev.arguments.push_back(L"SET"); - ev.arguments.push_back(key); - event_fire(&ev); + event_fire(event_t::variable(key, {L"VARIABLE", L"SET", key})); react_to_variable_change(L"SET", key, *this); return ENV_OK; } @@ -1327,12 +1317,7 @@ int env_stack_t::remove(const wcstring &key, int var_mode) { } if (try_remove(first_node, key.c_str(), var_mode)) { - event_t ev = event_t::variable_event(key); - ev.arguments.push_back(L"VARIABLE"); - ev.arguments.push_back(L"ERASE"); - ev.arguments.push_back(key); - event_fire(&ev); - + event_fire(event_t::variable(key, {L"VARIABLE", L"ERASE", key})); erased = 1; } } @@ -1347,11 +1332,7 @@ int env_stack_t::remove(const wcstring &key, int var_mode) { } if (erased) { env_universal_barrier(); - event_t ev = event_t::variable_event(key); - ev.arguments.push_back(L"VARIABLE"); - ev.arguments.push_back(L"ERASE"); - ev.arguments.push_back(key); - event_fire(&ev); + event_fire(event_t::variable(key, {L"VARIABLE", L"ERASE", key})); } if (is_exported) vars_stack().mark_changed_exported(); diff --git a/src/event.cpp b/src/event.cpp index 9ebf3b1dc..c345edd03 100644 --- a/src/event.cpp +++ b/src/event.cpp @@ -82,12 +82,11 @@ public: static pending_signals_t s_pending_signals; -typedef std::vector> event_list_t; - /// List of event handlers. -static event_list_t s_event_handlers; +static event_handler_list_t s_event_handlers; /// List of events that have been sent but have not yet been delivered because they are blocked. +using event_list_t = std::vector>; static event_list_t blocked; /// Variables (one per signal) set when a signal is observed. This is inspected by a signal handler. @@ -99,44 +98,33 @@ static void set_signal_observed(int sig, bool val) { } } -/// Tests if one event instance matches the definition of a event class. If both the class and the -/// instance name a function, they must name the same function. -static int event_match(const event_t &classv, const event_t &instance) { - // If the function names are both non-empty and different, then it's not a match. - if (!classv.function_name.empty() && !instance.function_name.empty() && - classv.function_name != instance.function_name) { - return 0; - } +/// Tests if one event instance matches the definition of a event class. +static bool handler_matches(const event_handler_t &classv, const event_t &instance) { + if (classv.desc.type == event_type_t::any) return true; + if (classv.desc.type != instance.desc.type) return false; - if (classv.type == event_type_t::any) return 1; - if (classv.type != instance.type) return 0; - - switch (classv.type) { + switch (classv.desc.type) { case event_type_t::signal: { - return classv.param1.signal == instance.param1.signal; + return classv.desc.param1.signal == instance.desc.param1.signal; } case event_type_t::variable: { - return instance.str_param1 == classv.str_param1; + return instance.desc.str_param1 == classv.desc.str_param1; } case event_type_t::exit: { - if (classv.param1.pid == EVENT_ANY_PID) return 1; - return classv.param1.pid == instance.param1.pid; + if (classv.desc.param1.pid == EVENT_ANY_PID) return true; + return classv.desc.param1.pid == instance.desc.param1.pid; } case event_type_t::job_exit: { - return classv.param1.job_id == instance.param1.job_id; + return classv.desc.param1.job_id == instance.desc.param1.job_id; } case event_type_t::generic: { - return instance.str_param1 == classv.str_param1; + return classv.desc.str_param1 == instance.desc.str_param1; } default: { DIE("unexpected classv.type"); - break; + return false; } } - - // This should never be reached. - debug(0, "Warning: Unreachable code reached in event_match in event.cpp\n"); - return 0; } /// Test if specified event is blocked. @@ -151,54 +139,52 @@ static int event_is_blocked(const event_t &e) { return event_block_list_blocks_type(parser.global_event_blocks); } -wcstring event_get_desc(const event_t &e) { - wcstring result; - switch (e.type) { +wcstring event_get_desc(const event_t &evt) { + const event_description_t &ed = evt.desc; + switch (ed.type) { case event_type_t::signal: { - result = format_string(_(L"signal handler for %ls (%ls)"), sig2wcs(e.param1.signal), - signal_get_desc(e.param1.signal)); - break; + return format_string(_(L"signal handler for %ls (%ls)"), sig2wcs(ed.param1.signal), + signal_get_desc(ed.param1.signal)); } + case event_type_t::variable: { - result = format_string(_(L"handler for variable '%ls'"), e.str_param1.c_str()); - break; + return format_string(_(L"handler for variable '%ls'"), ed.str_param1.c_str()); } + case event_type_t::exit: { - if (e.param1.pid > 0) { - result = format_string(_(L"exit handler for process %d"), e.param1.pid); + if (ed.param1.pid > 0) { + return format_string(_(L"exit handler for process %d"), ed.param1.pid); } else { // In events, PGIDs are stored as negative PIDs - job_t *j = job_t::from_pid(-e.param1.pid); - if (j) - result = format_string(_(L"exit handler for job %d, '%ls'"), j->job_id, - j->command_wcstr()); - else - result = format_string(_(L"exit handler for job with process group %d"), - -e.param1.pid); + job_t *j = job_t::from_pid(-ed.param1.pid); + if (j) { + return format_string(_(L"exit handler for job %d, '%ls'"), j->job_id, + j->command_wcstr()); + } else { + return format_string(_(L"exit handler for job with process group %d"), + -ed.param1.pid); + } } - break; + assert(0 && "Unreachable"); } - case event_type_t::job_exit: { - job_t *j = job_t::from_job_id(e.param1.job_id); - if (j) { - result = format_string(_(L"exit handler for job %d, '%ls'"), j->job_id, - j->command_wcstr()); - } else { - result = format_string(_(L"exit handler for job with job id %d"), e.param1.job_id); - } - break; - } - case event_type_t::generic: { - result = format_string(_(L"handler for generic event '%ls'"), e.str_param1.c_str()); - break; - } - default: { - result = format_string(_(L"Unknown event type '0x%x'"), e.type); - break; - } - } - return result; + case event_type_t::job_exit: { + job_t *j = job_t::from_job_id(ed.param1.job_id); + if (j) { + return format_string(_(L"exit handler for job %d, '%ls'"), j->job_id, + j->command_wcstr()); + } else { + return format_string(_(L"exit handler for job with job id %d"), ed.param1.job_id); + } + break; + } + + case event_type_t::generic: { + return format_string(_(L"handler for generic event '%ls'"), ed.str_param1.c_str()); + } + default: + assert(0 && "Unknown event type"); + } } #if 0 @@ -212,119 +198,34 @@ static void show_all_handlers(void) { } #endif -/// Give a more condensed description of \c event compared to \c event_get_desc. It includes what -/// function will fire if the \c event is an event handler. -static wcstring event_desc_compact(const event_t &event) { - wcstring res; - switch (event.type) { - case event_type_t::any: { - res = L"EVENT_ANY"; - break; - } - case event_type_t::variable: { - if (event.str_param1.c_str()) { - res = format_string(L"EVENT_VARIABLE($%ls)", event.str_param1.c_str()); - } else { - res = L"EVENT_VARIABLE([any])"; - } - break; - } - case event_type_t::signal: { - res = format_string(L"EVENT_SIGNAL(%ls)", sig2wcs(event.param1.signal)); - break; - } - case event_type_t::exit: { - if (event.param1.pid == EVENT_ANY_PID) { - res = wcstring(L"EVENT_EXIT([all child processes])"); - } else if (event.param1.pid > 0) { - res = format_string(L"EVENT_EXIT(pid %d)", event.param1.pid); - } else { - // In events, PGIDs are stored as negative PIDs - job_t *j = job_t::from_pid(-event.param1.pid); - if (j) - res = format_string(L"EVENT_EXIT(jobid %d: \"%ls\")", j->job_id, - j->command_wcstr()); - else - res = format_string(L"EVENT_EXIT(pgid %d)", -event.param1.pid); - } - break; - } - case event_type_t::job_exit: { - job_t *j = job_t::from_job_id(event.param1.job_id); - if (j) - res = - format_string(L"EVENT_JOB_ID(job %d: \"%ls\")", j->job_id, j->command_wcstr()); - else - res = format_string(L"EVENT_JOB_ID(jobid %d)", event.param1.job_id); - break; - } - case event_type_t::generic: { - res = format_string(L"EVENT_GENERIC(%ls)", event.str_param1.c_str()); - break; - } - default: { - res = format_string(L"unknown/illegal event(%x)", event.type); - break; - } +void event_add_handler(std::shared_ptr eh) { + if (eh->desc.type == event_type_t::signal) { + signal_handle(eh->desc.param1.signal, 1); + set_signal_observed(eh->desc.param1.signal, true); } - if (event.function_name.size()) { - return format_string(L"%ls: \"%ls\"", res.c_str(), event.function_name.c_str()); - } - return res; + + s_event_handlers.push_back(std::move(eh)); } -void event_add_handler(const event_t &event) { - if (debug_level >= 3) { - wcstring desc = event_desc_compact(event); - debug(3, "register: %ls", desc.c_str()); - } - - shared_ptr e = std::make_shared(event); - if (e->type == event_type_t::signal) { - signal_handle(e->param1.signal, 1); - set_signal_observed(e->param1.signal, true); - } - - s_event_handlers.push_back(std::move(e)); -} - -void event_remove(const event_t &criterion) { - if (debug_level >= 3) { - wcstring desc = event_desc_compact(criterion); - debug(3, "unregister: %ls", desc.c_str()); - } - - event_list_t::iterator iter = s_event_handlers.begin(); - while (iter != s_event_handlers.end()) { - const event_t *n = iter->get(); - if (!event_match(criterion, *n)) { - ++iter; - continue; - } - - // If this event was a signal handler and no other handler handles the specified signal - // type, do not handle that type of signal any more. - if (n->type == event_type_t::signal) { - event_t e = event_t::signal_event(n->param1.signal); - if (event_get(e, NULL) == 1) { - signal_handle(e.param1.signal, 0); - set_signal_observed(e.param1.signal, 0); - } - } - iter = s_event_handlers.erase(iter); - } -} - -int event_get(const event_t &criterion, event_list_t *out) { +void event_remove_function_handlers(const wcstring &name) { ASSERT_IS_MAIN_THREAD(); - int found = 0; - for (const shared_ptr &n : s_event_handlers) { - if (event_match(criterion, *n)) { - found++; - if (out) out->push_back(n); + auto begin = s_event_handlers.begin(), end = s_event_handlers.end(); + s_event_handlers.erase(std::remove_if(begin, end, + [&](const shared_ptr &eh) { + return eh->function_name == name; + }), + end); +} + +event_handler_list_t event_get_function_handlers(const wcstring &name) { + ASSERT_IS_MAIN_THREAD(); + event_handler_list_t result; + for (const shared_ptr &eh : s_event_handlers) { + if (eh->function_name == name) { + result.push_back(eh); } } - return found; + return result; } bool event_is_signal_observed(int sig) { @@ -344,14 +245,12 @@ static void event_fire_internal(const event_t &event) { assert(is_event >= 0 && "is_event should not be negative"); scoped_push inc_event{&is_event, is_event + 1}; - // Iterate over all events, adding events that should be fired to a second list. We need - // to do this in a separate step since an event handler might call event_remove or - // event_add_handler, which will change the contents of the \c events list. - event_list_t fire; - for (shared_ptr &criterion : s_event_handlers) { + // Capture the event handlers that match this event. + event_handler_list_t fire; + for (const auto &handler : s_event_handlers) { // Check if this event is a match. - if (event_match(*criterion, event)) { - fire.push_back(criterion); + if (handler_matches(*handler, event)) { + fire.push_back(handler); } } @@ -359,29 +258,27 @@ static void event_fire_internal(const event_t &event) { if (fire.empty()) return; if (signal_is_blocked()) { - // Fix for https://github.com/fish-shell/fish-shell/issues/608. Don't run event handlers - // while signals are blocked. + // Fix for #608. Don't run event handlers while signals are blocked. input_common_add_callback([event]() { ASSERT_IS_MAIN_THREAD(); - event_fire(&event); + event_fire(event); }); return; } - // Iterate over our list of matching events. - for (shared_ptr &criterion : fire) { + // Iterate over our list of matching events. Fire the ones that are still present. + for (const shared_ptr &handler : fire) { // Only fire if this event is still present - if (!contains(s_event_handlers, criterion)) { + if (!contains(s_event_handlers, handler)) { continue; } - // Fire event. - wcstring buffer = criterion->function_name; - - for (size_t j = 0; j < event.arguments.size(); j++) { - wcstring arg_esc = escape_string(event.arguments.at(j), 1); - buffer += L" "; - buffer += arg_esc; + // Construct a buffer to evaluate, starting with the function name and then all the + // arguments. + wcstring buffer = handler->function_name; + for (const wcstring &arg : event.arguments) { + buffer.push_back(L' '); + buffer.append(escape_string(arg, ESCAPE_ALL)); } // debug( 1, L"Event handler fires command '%ls'", buffer.c_str() ); @@ -401,7 +298,7 @@ static void event_fire_internal(const event_t &event) { } /// Handle all pending signal events. -static void event_fire_delayed() { +void event_fire_delayed() { ASSERT_IS_MAIN_THREAD(); // Do not invoke new event handlers from within event handlers. if (is_event) @@ -417,7 +314,7 @@ static void event_fire_delayed() { for (uint32_t sig=0; sig < signals.size(); sig++) { if (signals.test(sig)) { auto e = std::make_shared(event_type_t::signal); - e->param1.signal = sig; + e->desc.param1.signal = sig; e->arguments.push_back(sig2wcs(sig)); to_send.push_back(std::move(e)); } @@ -439,16 +336,14 @@ void event_enqueue_signal(int signal) { s_pending_signals.mark(signal); } -void event_fire(const event_t *event) { +void event_fire(const event_t &event) { // Fire events triggered by signals. event_fire_delayed(); - if (event) { - if (event_is_blocked(*event)) { - blocked.push_back(std::make_shared(*event)); - } else { - event_fire_internal(*event); - } + if (event_is_blocked(event)) { + blocked.push_back(std::make_shared(event)); + } else { + event_fire_internal(event); } } @@ -485,52 +380,54 @@ static const wchar_t *event_name_for_type(event_type_t type) { void event_print(io_streams_t &streams, maybe_t type_filter) { - std::vector> tmp = s_event_handlers; + event_handler_list_t tmp = s_event_handlers; std::sort(tmp.begin(), tmp.end(), - [](const shared_ptr &e1, const shared_ptr &e2) { - if (e1->type == e2->type) { - switch (e1->type) { - case event_type_t::signal: - return e1->param1.signal < e2->param1.signal; - case event_type_t::exit: - return e1->param1.pid < e2->param1.pid; - case event_type_t::job_exit: - return e1->param1.job_id < e2->param1.job_id; - case event_type_t::variable: - case event_type_t::any: - case event_type_t::generic: - return e1->str_param1 < e2->str_param1; - } - } - return e1->type < e2->type; - }); + [](const shared_ptr &e1, const shared_ptr &e2) { + const event_description_t &d1 = e1->desc; + const event_description_t &d2 = e2->desc; + if (d1.type != d2.type) { + return d1.type < d2.type; + } + switch (d1.type) { + case event_type_t::signal: + return d1.signal < d2.signal; + case event_type_t::exit: + return d1.param1.pid < d2.param1.pid; + case event_type_t::job_exit: + return d1.param1.job_id < d2.param1.job_id; + case event_type_t::variable: + case event_type_t::any: + case event_type_t::generic: + return d1.str_param1 < d2.str_param1; + } + }); maybe_t last_type{}; - for (const shared_ptr &evt : tmp) { + for (const shared_ptr &evt : tmp) { // If we have a filter, skip events that don't match. - if (type_filter && *type_filter != evt->type) { + if (type_filter && *type_filter != evt->desc.type) { continue; } - if (!last_type || *last_type != evt->type) { + if (!last_type || *last_type != evt->desc.type) { if (last_type) streams.out.append(L"\n"); - last_type = static_cast(evt->type); + last_type = static_cast(evt->desc.type); streams.out.append_format(L"Event %ls\n", event_name_for_type(*last_type)); } - switch (evt->type) { + switch (evt->desc.type) { case event_type_t::signal: - streams.out.append_format(L"%ls %ls\n", sig2wcs(evt->param1.signal), - evt->function_name.c_str()); + streams.out.append_format(L"%ls %ls\n", sig2wcs(evt->desc.param1.signal), + evt->function_name.c_str()); break; case event_type_t::job_exit: - streams.out.append_format(L"%d %ls\n", evt->param1, - evt->function_name.c_str()); + streams.out.append_format(L"%d %ls\n", evt->desc.param1, + evt->function_name.c_str()); break; case event_type_t::variable: case event_type_t::generic: - streams.out.append_format(L"%ls %ls\n", evt->str_param1.c_str(), - evt->function_name.c_str()); + streams.out.append_format(L"%ls %ls\n", evt->desc.str_param1.c_str(), + evt->function_name.c_str()); break; default: streams.out.append_format(L"%ls\n", evt->function_name.c_str()); @@ -539,33 +436,36 @@ void event_print(io_streams_t &streams, maybe_t type_filter) { } } -void event_fire_generic(const wchar_t *name, wcstring_list_t *args) { +void event_fire_generic(const wchar_t *name, const wcstring_list_t *args) { CHECK(name, ); event_t ev(event_type_t::generic); - ev.str_param1 = name; + ev.desc.str_param1 = name; if (args) ev.arguments = *args; - event_fire(&ev); + event_fire(ev); } -event_t::event_t(event_type_t t) : type(t), param1(), str_param1(), function_name(), arguments() {} - -event_t::~event_t() = default; - -event_t event_t::signal_event(int sig) { - event_t event(event_type_t::signal); +event_description_t event_description_t::signal(int sig) { + event_description_t event(event_type_t::signal); event.param1.signal = sig; return event; } -event_t event_t::variable_event(const wcstring &str) { - event_t event(event_type_t::variable); - event.str_param1 = str; +event_description_t event_description_t::variable(wcstring str) { + event_description_t event(event_type_t::variable); + event.str_param1 = std::move(str); return event; } -event_t event_t::generic_event(const wcstring &str) { - event_t event(event_type_t::generic); - event.str_param1 = str; +event_description_t event_description_t::generic(wcstring str) { + event_description_t event(event_type_t::generic); + event.str_param1 = std::move(str); return event; } + +event_t event_t::variable(wcstring name, wcstring_list_t args) { + event_t evt{event_type_t::variable}; + evt.desc.str_param1 = std::move(name); + evt.arguments = std::move(args); + return evt; +} diff --git a/src/event.h b/src/event.h index a56ee98ce..7fa7baf8f 100644 --- a/src/event.h +++ b/src/event.h @@ -35,18 +35,9 @@ enum class event_type_t { generic, }; -/// The structure which represents an event. The event_t struct has several event-related use-cases: -/// -/// - When used as a parameter to event_add, it represents a class of events, and function_name is -/// the name of the function which will be called whenever an event matching the specified class -/// occurs. This is also how events are stored internally. -/// -/// - When used as a parameter to event_get, event_remove and event_fire, it represents a class of -/// events, and if the function_name field is non-zero, only events which call the specified -/// function will be returned. -struct event_t { - public: - /// Type of event. +/// Properties of an event. +struct event_description_t { + /// The event type. event_type_t type; /// The type-specific parameter. The int types are one of the following: @@ -58,67 +49,65 @@ struct event_t { int signal; int job_id; pid_t pid; - } param1; + } param1{}; /// The string types are one of the following: /// /// variable: Variable name for variable-type events. /// param: The parameter describing this generic event. - wcstring str_param1; + wcstring str_param1{}; - /// The name of the event handler function. - wcstring function_name; + explicit event_description_t(event_type_t t) : type(t) {} + static event_description_t signal(int sig); + static event_description_t variable(wcstring str); + static event_description_t generic(wcstring str); +}; - /// The argument list. Only used when sending a new event using event_fire. In all other - /// situations, the value of this variable is ignored. - wcstring_list_t arguments; +/// Represents a handler for an event. +struct event_handler_t { + /// Properties of the event to match. + event_description_t desc; - explicit event_t(event_type_t t); - ~event_t(); + /// Name of the function to invoke. + wcstring function_name{}; - static event_t signal_event(int sig); - static event_t variable_event(const wcstring &str); - static event_t generic_event(const wcstring &str); + explicit event_handler_t(event_type_t t) : desc(t) {} + event_handler_t(event_description_t d, wcstring name) + : desc(d), function_name(std::move(name)) {} +}; +using event_handler_list_t = std::vector>; + +/// Represents a event that is fired, or capable of being fired. +struct event_t { + /// Properties of the event. + event_description_t desc; + + /// Arguments to any handler. + wcstring_list_t arguments{}; + + event_t(event_type_t t) : desc(t) {} + + static event_t variable(wcstring name, wcstring_list_t args); }; /// Add an event handler. -/// -/// May not be called by a signal handler, since it may allocate new memory. -void event_add_handler(const event_t &event); +void event_add_handler(std::shared_ptr eh); -/// Remove all events matching the specified criterion. -/// -/// May not be called by a signal handler, since it may free allocated memory. -void event_remove(const event_t &event); +/// Remove all events for the given function name. +void event_remove_function_handlers(const wcstring &name); -/// Return all events which match the specified event class -/// -/// This function is safe to call from a signal handler _ONLY_ if the out parameter is null. -/// -/// \param criterion Is the class of events to return. If the criterion has a non-null -/// function_name, only events which trigger the specified function will return. -/// \param out the list to add events to. May be 0, in which case no events will be added, but the -/// result count will still be valid -/// -/// \return the number of found matches -int event_get(const event_t &criterion, std::vector> *out); +/// Return all event handlers for the given function. +event_handler_list_t event_get_function_handlers(const wcstring &name); /// Returns whether an event listener is registered for the given signal. This is safe to call from /// a signal handler. bool event_is_signal_observed(int signal); -/// Fire the specified event. The function_name field of the event must be set to 0. If the event is -/// of type EVENT_SIGNAL, no the event is queued, and will be dispatched the next time event_fire is -/// called. If event is a null-pointer, all pending events are dispatched. -/// -/// This function is safe to call from a signal handler _ONLY_ if the event parameter is for a -/// signal. Signal events not be fired, by the call to event_fire, instead they will be fired the -/// next time event_fire is called with anull argument. This is needed to make sure that no code -/// evaluation is ever performed by a signal handler. -/// -/// \param event the specific event whose handlers should fire. If null, then all delayed events -/// will be fired. -void event_fire(const event_t *event); +/// Fire the specified event \p event. +void event_fire(const event_t &event); + +/// Fire all delayed eents. +void event_fire_delayed(); /// Enqueue a signal event. Invoked from a signal handler. void event_enqueue_signal(int signal); @@ -130,7 +119,7 @@ void event_print(io_streams_t &streams, maybe_t type_filter); wcstring event_get_desc(const event_t &e); /// Fire a generic event with the specified name. -void event_fire_generic(const wchar_t *name, wcstring_list_t *args = NULL); +void event_fire_generic(const wchar_t *name, const wcstring_list_t *args = NULL); /// Return the event type for a given name, or none. maybe_t event_type_for_name(const wcstring &name); diff --git a/src/function.cpp b/src/function.cpp index 1fae86831..bffa0d8cf 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -175,8 +175,8 @@ void function_add(const function_data_t &data, const parser_t &parser) { loaded_functions.insert(new_pair); // Add event handlers. - for (const event_t &event : data.events) { - event_add_handler(event); + for (const event_description_t &ed : data.events) { + event_add_handler(std::make_shared(ed, data.name)); } } @@ -224,9 +224,7 @@ static bool function_remove_ignore_autoload(const wcstring &name, bool tombstone if (iter->second.is_autoload && tombstone) function_tombstones.insert(name); loaded_functions.erase(iter); - event_t ev(event_type_t::any); - ev.function_name = name; - event_remove(ev); + event_remove_function_handlers(name); return true; } diff --git a/src/function.h b/src/function.h index 9ee7df6eb..fc6529e72 100644 --- a/src/function.h +++ b/src/function.h @@ -44,7 +44,7 @@ struct function_data_t { /// Function's metadata fields function_properties_t props; /// List of all event handlers for this function. - std::vector events; + std::vector events; }; /// Add a function. diff --git a/src/input.cpp b/src/input.cpp index 0a9e9a008..bcbc04fab 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -252,7 +252,7 @@ void input_mapping_add(const wchar_t *sequence, const wchar_t *command, const wc /// reader. static int interrupt_handler() { // Fire any pending events. - event_fire(NULL); + event_fire_delayed(); // Reap stray processes, including printing exit status messages. if (job_reap(true)) reader_repaint_needed(); // Tell the reader an event occured. diff --git a/src/proc.cpp b/src/proc.cpp index 9baa1bb00..81c7d4cab 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -106,9 +106,6 @@ void set_proc_had_barrier(bool flag) { proc_had_barrier = flag; } -/// The event variable used to send all process event. -static event_t event{event_type_t::any}; - /// A stack containing the values of is_interactive. Used by proc_push_interactive and /// proc_pop_interactive. static std::vector interactive_stack; @@ -487,14 +484,13 @@ static void print_job_status(const job_t *j, job_status_t status) { } void proc_fire_event(const wchar_t *msg, event_type_t type, pid_t pid, int status) { - event.type = type; - event.param1.pid = pid; + event_t event{type}; + event.desc.param1.pid = pid; event.arguments.push_back(msg); event.arguments.push_back(to_string(pid)); event.arguments.push_back(to_string(status)); - event_fire(&event); - event.arguments.resize(0); + event_fire(event); } static bool process_clean_after_marking(bool allow_interactive) {