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

@@ -29,7 +29,7 @@ struct function_cmd_opts_t {
     bool print_help = false;
     bool shadow_scope = true;
     wcstring description = L"";
-    std::vector<event_t> events;
+    std::vector<event_description_t> 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);

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<std::shared_ptr<event_t>> ev;
-    event_get(search, &ev);
+    std::vector<std::shared_ptr<event_handler_t>> 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: {

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();

src/event.cpp

@@ -82,12 +82,11 @@ public:
 
 static pending_signals_t s_pending_signals;
 
-typedef std::vector<shared_ptr<event_t>> 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<shared_ptr<event_t>>;
 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;
-        }
-        case event_type_t::variable: {
-            result = format_string(_(L"handler for variable '%ls'"), e.str_param1.c_str());
-            break;
-        }
-        case event_type_t::exit: {
-            if (e.param1.pid > 0) {
-                result = format_string(_(L"exit handler for process %d"), e.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);
-            }
-            break;
-        }
-        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 format_string(_(L"signal handler for %ls (%ls)"), sig2wcs(ed.param1.signal),
+                                 signal_get_desc(ed.param1.signal));
         }
 
-    return result;
+        case event_type_t::variable: {
+            return format_string(_(L"handler for variable '%ls'"), ed.str_param1.c_str());
+        }
+
+        case event_type_t::exit: {
+            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(-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);
+                }
+            }
+            assert(0 && "Unreachable");
+        }
+
+        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;
-        }
-    }
-    if (event.function_name.size()) {
-        return format_string(L"%ls: \"%ls\"", res.c_str(), event.function_name.c_str());
-    }
-    return res;
+void event_add_handler(std::shared_ptr<event_handler_t> eh) {
+    if (eh->desc.type == event_type_t::signal) {
+        signal_handle(eh->desc.param1.signal, 1);
+        set_signal_observed(eh->desc.param1.signal, true);
     }
 
-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());
+    s_event_handlers.push_back(std::move(eh));
 }
 
-    shared_ptr<event_t> e = std::make_shared<event_t>(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<event_t> &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<event_handler_t> &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<event_handler_t> &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<decltype(is_event)> 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<event_t> &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<event_t> &criterion : fire) {
+    // Iterate over our list of matching events. Fire the ones that are still present.
+    for (const shared_ptr<event_handler_t> &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_t>(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_t>(*event));
+    if (event_is_blocked(event)) {
+        blocked.push_back(std::make_shared<event_t>(event));
     } else {
-            event_fire_internal(*event);
-        }
+        event_fire_internal(event);
     }
 }
 
@@ -485,51 +380,53 @@ static const wchar_t *event_name_for_type(event_type_t type) {
 
 
 void event_print(io_streams_t &streams, maybe_t<event_type_t> type_filter) {
-    std::vector<shared_ptr<event_t>> tmp = s_event_handlers;
+    event_handler_list_t tmp = s_event_handlers;
     std::sort(tmp.begin(), tmp.end(),
-            [](const shared_ptr<event_t> &e1, const shared_ptr<event_t> &e2) {
-                if (e1->type == e2->type) {
-                    switch (e1->type) {
+              [](const shared_ptr<event_handler_t> &e1, const shared_ptr<event_handler_t> &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 e1->param1.signal < e2->param1.signal;
+                          return d1.signal < d2.signal;
                       case event_type_t::exit:
-                            return e1->param1.pid < e2->param1.pid;
+                          return d1.param1.pid < d2.param1.pid;
                       case event_type_t::job_exit:
-                            return e1->param1.job_id < e2->param1.job_id;
+                          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 e1->str_param1 < e2->str_param1;
+                          return d1.str_param1 < d2.str_param1;
                   }
-                }
-                return e1->type < e2->type;
               });
 
     maybe_t<event_type_t> last_type{};
-    for (const shared_ptr<event_t> &evt : tmp) {
+    for (const shared_ptr<event_handler_t> &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<event_type_t>(evt->type);
+            last_type = static_cast<event_type_t>(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),
+                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,
+                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(),
+                streams.out.append_format(L"%ls %ls\n", evt->desc.str_param1.c_str(),
                                           evt->function_name.c_str());
                 break;
             default:
@@ -539,33 +436,36 @@ void event_print(io_streams_t &streams, maybe_t<event_type_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;
+}

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<std::shared_ptr<event_handler_t>>;
+
+/// 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<event_handler_t> 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<std::shared_ptr<event_t>> *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<event_type_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_t> event_type_for_name(const wcstring &name);

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<event_handler_t>(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;
 }
 

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<event_t> events;
+    std::vector<event_description_t> events;
 };
 
 /// Add a function.

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.

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<int> 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) {