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 2 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
27 changes: 14 additions & 13 deletions src/eventloop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,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 +119,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 +139,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 +185,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,15 +194,17 @@ 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);

uv_poll_start(input_handle->handle, input_handle->cond, handle_input);
g_warning("input_add: produced %p (%u)", input_event, GPOINTER_TO_UINT(input_event));
return GPOINTER_TO_UINT(input_event);
}

Expand All @@ -215,24 +216,24 @@ guint input_add(int fd, PurpleInputCondition cond,
*/
gboolean input_remove (guint int_handle) {
gpointer handle = GUINT_TO_POINTER(int_handle);
g_warning("input_remove: removing %p (%u)", handle, int_handle);
g_return_val_if_fail(handle != NULL, false);
s_evLoopInputEvent *inputEvent = handle;
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 +257,7 @@ 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);
}
return &glib_eventloops;
}
Expand All @@ -271,7 +272,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