Clean up event_t handling

Use shared_ptr instead of the silly killme list
This commit is contained in:
ridiculousfish 2017-01-21 16:48:07 -08:00
parent 8a0d4854e8
commit f0065cda13
4 changed files with 51 additions and 100 deletions

View file

@ -944,7 +944,7 @@ static wcstring functions_def(const wcstring &name) {
function_get_definition(name, &def);
event_t search(EVENT_ANY);
search.function_name = name;
std::vector<event_t *> ev;
std::vector<std::shared_ptr<event_t>> ev;
event_get(search, &ev);
out.append(L"function ");
@ -967,7 +967,7 @@ static wcstring functions_def(const wcstring &name) {
}
for (size_t i = 0; i < ev.size(); i++) {
event_t *next = ev.at(i);
const event_t *next = ev.at(i).get();
switch (next->type) {
case EVENT_SIGNAL: {
append_format(out, L" --on-signal %ls", sig2wcs(next->param1.signal));

View file

@ -479,7 +479,6 @@ void complete_remove(const wcstring &cmd, bool cmd_is_path, const wcstring &opti
bool delete_it = entry.remove_option(option, type);
if (delete_it) {
// Delete this entry.
completion_set.erase(iter);
}
}

View file

@ -39,12 +39,10 @@ static signal_list_t sig_list[2] = {{}, {}};
/// The index of sig_list that is the list of signals currently written to.
static volatile int active_list = 0;
typedef std::vector<event_t *> event_list_t;
typedef std::vector<shared_ptr<event_t>> event_list_t;
/// List of event handlers.
static event_list_t s_event_handlers;
/// List of event handlers that should be removed.
static event_list_t killme;
/// List of events that have been sent but have not yet been delivered because they are blocked.
static event_list_t blocked;
@ -242,61 +240,51 @@ static wcstring event_desc_compact(const event_t &event) {
}
void event_add_handler(const event_t &event) {
event_t *e;
if (debug_level >= 3) {
wcstring desc = event_desc_compact(event);
debug(3, "register: %ls\n", desc.c_str());
}
e = new event_t(event);
shared_ptr<event_t> e = std::make_shared<event_t>(event);
if (e->type == EVENT_SIGNAL) {
signal_handle(e->param1.signal, 1);
set_signal_observed(e->param1.signal, true);
}
s_event_handlers.push_back(e);
s_event_handlers.push_back(std::move(e));
}
void event_remove(const event_t &criterion) {
event_list_t new_list;
if (debug_level >= 3) {
wcstring desc = event_desc_compact(criterion);
debug(3, "unregister: %ls\n", desc.c_str());
}
// Because of concurrency issues (env_remove could remove an event that is currently being
// executed), env_remove does not actually free any events - instead it simply moves all events
// that should be removed from the event list to the killme list, and the ones that shouldn't be
// killed to new_list, and then drops the empty events-list.
if (s_event_handlers.empty()) return;
for (size_t i = 0; i < s_event_handlers.size(); i++) {
event_t *n = s_event_handlers.at(i);
if (event_match(criterion, *n)) {
killme.push_back(n);
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_SIGNAL) {
event_t e = event_t::signal_event(n->param1.signal);
if (event_get(e, 0) == 1) {
if (event_get(e, NULL) == 1) {
signal_handle(e.param1.signal, 0);
set_signal_observed(e.param1.signal, 0);
}
}
} else {
new_list.push_back(n);
iter = s_event_handlers.erase(iter);
}
}
s_event_handlers.swap(new_list);
}
int event_get(const event_t &criterion, std::vector<event_t *> *out) {
int event_get(const event_t &criterion, event_list_t *out) {
ASSERT_IS_MAIN_THREAD();
int found = 0;
for (size_t i = 0; i < s_event_handlers.size(); i++) {
event_t *n = s_event_handlers.at(i);
for (const shared_ptr<event_t> &n : s_event_handlers) {
if (event_match(criterion, *n)) {
found++;
if (out) out->push_back(n);
@ -314,17 +302,6 @@ bool event_is_signal_observed(int sig) {
return result;
}
/// Free all events in the kill list.
static void event_free_kills() {
for_each(killme.begin(), killme.end(), event_free);
killme.resize(0);
}
/// Test if the specified event is waiting to be killed.
static int event_is_killed(const event_t &e) {
return std::find(killme.begin(), killme.end(), &e) != killme.end();
}
/// Callback for firing (and then deleting) an event.
static void fire_event_callback(void *arg) {
ASSERT_IS_MAIN_THREAD();
@ -338,20 +315,11 @@ static void fire_event_callback(void *arg) {
/// event handler, we make sure to optimize the 'no matches' path. This means that nothing is
/// allocated/initialized unless needed.
static void event_fire_internal(const event_t &event) {
event_list_t fire;
// First we free all events that have been removed, but only if this invocation of
// event_fire_internal is not a recursive call.
if (is_event <= 1) event_free_kills();
if (s_event_handlers.empty()) return;
// Then we iterate over all events, adding events that should be fired to a second list. We need
// 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.
for (size_t i = 0; i < s_event_handlers.size(); i++) {
event_t *criterion = s_event_handlers.at(i);
event_list_t fire;
for (shared_ptr<event_t> &criterion : s_event_handlers) {
// Check if this event is a match.
if (event_match(*criterion, event)) {
fire.push_back(criterion);
@ -370,12 +338,13 @@ static void event_fire_internal(const event_t &event) {
}
// Iterate over our list of matching events.
for (size_t i = 0; i < fire.size(); i++) {
event_t *criterion = fire.at(i);
int prev_status;
// Check if this event has been removed, if so, dont fire it.
if (event_is_killed(*criterion)) continue;
for (shared_ptr<event_t> &criterion : fire) {
// Only fire if this event is still present
if (std::find(s_event_handlers.begin(),
s_event_handlers.end(),
criterion) == s_event_handlers.end()) {
continue;
}
// Fire event.
wcstring buffer = criterion->function_name;
@ -391,7 +360,7 @@ static void event_fire_internal(const event_t &event) {
// Event handlers are not part of the main flow of code, so they are marked as
// non-interactive.
proc_push_interactive(0);
prev_status = proc_get_last_status();
int prev_status = proc_get_last_status();
parser_t &parser = parser_t::principal_parser();
event_block_t *b = parser.push_block<event_block_t>(event);
@ -400,9 +369,6 @@ static void event_fire_internal(const event_t &event) {
proc_pop_interactive();
proc_set_last_status(prev_status);
}
// Free killed events.
if (is_event <= 1) event_free_kills();
}
/// Handle all pending signal events.
@ -412,18 +378,15 @@ static void event_fire_delayed() {
// When the event handler has called a piece of code that triggers another event, we do not want
// to fire delayed events because of concurrency problems.
if (!blocked.empty() && is_event == 1) {
event_list_t new_blocked;
for (size_t i = 0; i < blocked.size(); i++) {
event_t *e = blocked.at(i);
event_list_t local_blocked;
local_blocked.swap(blocked);
for (const shared_ptr<event_t> &e : local_blocked) {
if (event_is_blocked(*e)) {
new_blocked.push_back(new event_t(*e));
blocked.push_back(e);
} else {
event_fire_internal(*e);
event_free(e);
}
}
blocked.swap(new_blocked);
}
int al = active_list;
@ -439,21 +402,20 @@ static void event_fire_delayed() {
// Set up.
lst = &sig_list[1 - al];
event_t e = event_t::signal_event(0);
e.arguments.resize(1);
if (lst->overflow) {
debug(0, _(L"Signal list overflow. Signals have been ignored."));
}
// Send all signals in our private list.
for (int i = 0; i < lst->count; i++) {
e.param1.signal = lst->signal[i];
e.arguments.at(0) = sig2wcs(e.param1.signal);
if (event_is_blocked(e)) {
blocked.push_back(new event_t(e));
shared_ptr<event_t> e = std::make_shared<event_t>(EVENT_SIGNAL);
int signal = lst->signal[i];
e->param1.signal = signal;
e->arguments.push_back(sig2wcs(signal));
if (event_is_blocked(*e)) {
blocked.push_back(e);
} else {
event_fire_internal(e);
event_fire_internal(*e);
}
}
}
@ -479,7 +441,7 @@ void event_fire(const event_t *event) {
if (event) {
if (event_is_blocked(*event)) {
blocked.push_back(new event_t(*event));
blocked.push_back(std::make_shared<event_t>(*event));
} else {
event_fire_internal(*event);
}
@ -491,16 +453,7 @@ void event_fire(const event_t *event) {
void event_init() {}
void event_destroy() {
for_each(s_event_handlers.begin(), s_event_handlers.end(), event_free);
s_event_handlers.clear();
for_each(killme.begin(), killme.end(), event_free);
killme.clear();
}
void event_free(event_t *e) {
CHECK(e, );
delete e;
}
void event_fire_generic(const wchar_t *name, wcstring_list_t *args) {

View file

@ -7,6 +7,7 @@
#define FISH_EVENT_H
#include <unistd.h>
#include <memory>
#include <vector>
#include "common.h"
@ -100,7 +101,7 @@ void event_remove(const event_t &event);
/// result count will still be valid
///
/// \return the number of found matches
int event_get(const event_t &criterion, std::vector<event_t *> *out);
int event_get(const event_t &criterion, std::vector<std::shared_ptr<event_t>> *out);
/// Returns whether an event listener is registered for the given signal. This is safe to call from
/// a signal handler.
@ -120,6 +121,7 @@ bool event_is_signal_observed(int signal);
void event_fire(const event_t *event);
/// Like event_fire, but takes a signal directly.
/// May be called from signal handlers
void event_fire_signal(int signal);
/// Initialize the event-handling library.
@ -128,9 +130,6 @@ void event_init();
/// Destroy the event-handling library.
void event_destroy();
/// Free all memory used by the specified event.
void event_free(event_t *e);
/// Returns a string describing the specified event.
wcstring event_get_desc(const event_t &e);