Skip to content

Crash less #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Crash less #8

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1cef2e5
Index inputs hashtable by the FD integer value rather than its address
tadzik Sep 14, 2021
45031b9
Make sure we don't free uv handles too early
tadzik Sep 15, 2021
9078216
Don't use s_evLoopInputEvent addresses as their libpurple IDs
tadzik Sep 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 24 additions & 18 deletions src/eventloop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ typedef struct {
} s_evLoopInput;

typedef struct {
guint id;
PurpleInputFunction func;
gpointer user_data;
int events;
Expand All @@ -39,6 +40,9 @@ typedef struct {
uv_loop_t* loop;
// fd -> s_evLoopInput
GHashTable* inputs;
guint last_event_id;
// guint -> s_evLoopInputEvent
GHashTable* events;
} s_evLoopState;

void call_callback(uv_timer_t* handle);
Expand Down Expand Up @@ -98,7 +102,7 @@ guint timeout_add_seconds(guint interval, GSourceFunc function, gpointer data) {
return timeout_add(interval*1000, function, data);
}

void on_timer_close_complete(uv_handle_t* handle)
void on_handle_close_complete(uv_handle_t* handle)
{
free(handle->data);
free(handle);
Expand All @@ -119,7 +123,7 @@ gboolean timeout_remove(guint int_handle) {
s_evLoopTimer *timer = handle;
uv_timer_stop(timer->handle);
if (!uv_is_closing((uv_handle_t*)timer->handle)) {
uv_close((uv_handle_t*)timer->handle, on_timer_close_complete);
uv_close((uv_handle_t*)timer->handle, on_handle_close_complete);
}
return true;
}
Expand All @@ -139,7 +143,6 @@ void handle_input(uv_poll_t* handle, int status, int events) {
// Unexpected positive status
g_warning("handle_input unexpected positive status %i\n", status);
}
int closedFD = -1;
s_evLoopInput *input = handle->data;
GList *elem;
s_evLoopInputEvent *inputEvent;
Expand Down Expand Up @@ -186,7 +189,7 @@ guint input_add(int fd, PurpleInputCondition cond,
input_event->func = func;
input_event->user_data = user_data;

s_evLoopInput *input_handle = g_hash_table_lookup(evLoopState.inputs, &fd);
s_evLoopInput *input_handle = g_hash_table_lookup(evLoopState.inputs, GINT_TO_POINTER(fd));
if (input_handle == NULL) {
input_handle = g_malloc(sizeof(s_evLoopInput));
input_handle->fd = fd;
Expand All @@ -195,16 +198,20 @@ guint input_add(int fd, PurpleInputCondition cond,
input_handle->events = NULL;
uv_handle_set_data((uv_handle_t*)input_handle->handle, input_handle);
uv_poll_init(evLoopState.loop, input_handle->handle, fd);
g_hash_table_insert(evLoopState.inputs, &input_handle->fd, input_handle);
g_hash_table_insert(evLoopState.inputs, GINT_TO_POINTER(fd), input_handle);
} else {
// Nothing to do, except update the condition on the poll
input_handle->cond |= cond;
}
// This will update the handle if the cond changed.
uv_poll_start(input_handle->handle, input_handle->cond, handle_input);
input_event->parent = input_handle;
input_handle->events = g_list_append(input_handle->events, input_event);
return GPOINTER_TO_UINT(input_event);

input_event->id = evLoopState.last_event_id++;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably check if we didn't overflow here – how knows for how many decades it can run now that it got as stable as it is ;)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can check if evLoopState.last_event_id is maxint if you like, but honestly...meeeh

g_hash_table_insert(evLoopState.events, GUINT_TO_POINTER(input_event->id), input_event);

uv_poll_start(input_handle->handle, input_handle->cond, handle_input);
return input_event->id;
}

/**
Expand All @@ -213,26 +220,24 @@ guint input_add(int fd, PurpleInputCondition cond,
* @return @c TRUE if the input handler was found and removed.
* @see purple_input_remove
*/
gboolean input_remove (guint int_handle) {
gpointer handle = GUINT_TO_POINTER(int_handle);
g_return_val_if_fail(handle != NULL, false);
s_evLoopInputEvent *inputEvent = handle;
gboolean input_remove (guint input_event_id) {
s_evLoopInputEvent *inputEvent = g_hash_table_lookup(evLoopState.events, GUINT_TO_POINTER(input_event_id));
g_return_val_if_fail(inputEvent != NULL, false);
s_evLoopInput *input = inputEvent->parent;
// Why is this here? XXX
if (g_list_find(input->events, inputEvent) == NULL) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is used to check if the input actually exists in our hash, but granted it's probably pointless here.

return false;
}
input->events = g_list_remove(input->events, inputEvent);
free(inputEvent);
g_free(inputEvent);
guint listeners = g_list_length(input->events);
if (listeners > 0) {
// TODO: We should change the flags for the poll handle here.
return true;
// Do not clean up the handle yet.
}
uv_poll_stop(input->handle);
g_hash_table_remove(evLoopState.inputs, &input->fd);
free(input->handle);
free(input);
uv_close((uv_handle_t*)input->handle, on_handle_close_complete);
g_hash_table_remove(evLoopState.inputs, GINT_TO_POINTER(input->fd));
return true;
}

Expand All @@ -256,7 +261,8 @@ PurpleEventLoopUiOps* eventLoop_get(napi_env* env) {
if (napi_get_uv_event_loop(*env, &evLoopState.loop) != napi_ok) {
THROW(*env, NULL, "Could not get UV loop", NULL);
}
evLoopState.inputs = g_hash_table_new_full(g_int_hash, g_int_equal, NULL, NULL);
evLoopState.inputs = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, NULL);
evLoopState.events = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, NULL);
}
return &glib_eventloops;
}
Expand All @@ -271,7 +277,7 @@ void call_callback(uv_timer_t* handle) {
gboolean res = timer->function(timer->data);
// If the function succeeds, continue
if (!res && !uv_is_closing((uv_handle_t *)timer->handle)) {
uv_close((uv_handle_t *)timer->handle, on_timer_close_complete);
uv_close((uv_handle_t *)timer->handle, on_handle_close_complete);
return;
}
uv_timer_again(handle);
Expand Down