From 0a87e43aff0346546e10d1ec6523ad7552dfddc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emysl=20Janouch?= Date: Tue, 21 Apr 2015 00:04:34 +0200 Subject: [PATCH] degesch: unfuck reference counting We really needed weak references for the name map. --- degesch.c | 107 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 65 insertions(+), 42 deletions(-) diff --git a/degesch.c b/degesch.c index 1722e40..a18afea 100644 --- a/degesch.c +++ b/degesch.c @@ -103,6 +103,9 @@ static struct config_item g_config_table[] = static void user_unref (void *p); static void channel_unref (void *p); +/// Callback just before a reference counted object is destroyed +typedef void (*destroy_cb_fn) (void *object, void *user_data); + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - struct user_channel @@ -130,12 +133,15 @@ user_channel_destroy (void *p) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // We keep references to user information in channels and buffers, -// as well as in the name lookup table. +// and weak references in the name lookup table. struct user { size_t ref_count; ///< Reference count + destroy_cb_fn on_destroy; ///< To remove any weak references + void *user_data; ///< User data for callbacks + // TODO: eventually a reference to the server char *nickname; ///< Literal nickname @@ -156,6 +162,9 @@ user_new (void) static void user_destroy (struct user *self) { + if (self->on_destroy) + self->on_destroy (self, self->user_data); + free (self->nickname); LIST_FOR_EACH (struct user_channel, iter, self->channels) user_channel_destroy (iter); @@ -212,6 +221,9 @@ struct channel { size_t ref_count; ///< Reference count + destroy_cb_fn on_destroy; ///< To remove any weak references + void *user_data; ///< User data for callbacks + // TODO: eventually a reference to the server char *name; ///< Channel name @@ -230,9 +242,11 @@ channel_new (void) } static void -channel_destroy (void *p) +channel_destroy (struct channel *self) { - struct channel *self = p; + if (self->on_destroy) + self->on_destroy (self, self->user_data); + free (self->name); free (self->mode); free (self->topic); @@ -409,8 +423,6 @@ struct app_context // maybe also broadcast all buffers about the disconnection event // TODO: when getting connected again, rejoin all current channels - // FIXME: these should only have weak references, so that we don't have - // to watch all places where they might get unref-ed struct str_map irc_users; ///< IRC user data struct str_map irc_channels; ///< IRC channel data struct str_map irc_buffer_map; ///< Maps IRC identifiers to buffers @@ -480,10 +492,8 @@ app_context_init (struct app_context *self) self->irc_ready = false; str_map_init (&self->irc_users); - self->irc_users.free = user_unref; self->irc_users.key_xfrm = irc_strxfrm; str_map_init (&self->irc_channels); - self->irc_channels.free = channel_unref; self->irc_channels.key_xfrm = irc_strxfrm; str_map_init (&self->irc_buffer_map); self->irc_buffer_map.key_xfrm = irc_strxfrm; @@ -1057,13 +1067,6 @@ buffer_remove (struct app_context *ctx, struct buffer *buffer) } #endif // RL_READLINE_VERSION - // Get rid of the channel, as well as the users, - // if the buffer was the last thing keeping them alive - if (buffer->channel && buffer->channel->ref_count == 2) - str_map_set (&ctx->irc_channels, buffer->channel->name, NULL); - if (buffer->user && buffer->user->ref_count == 2) - str_map_set (&ctx->irc_users, buffer->user->nickname, NULL); - // And make sure to unlink the buffer from "irc_buffer_map" if (buffer->channel) str_map_set (&ctx->irc_buffer_map, buffer->channel->name, NULL); @@ -1284,6 +1287,50 @@ init_buffers (struct app_context *ctx) // --- Users, channels --------------------------------------------------------- +static void +irc_user_on_destroy (void *object, void *user_data) +{ + struct user *user = object; + struct app_context *ctx = user_data; + str_map_set (&ctx->irc_users, user->nickname, NULL); +} + +struct user * +irc_make_user (struct app_context *ctx, char *nickname) +{ + hard_assert (!str_map_find (&ctx->irc_users, nickname)); + + struct user *user = user_new (); + user->on_destroy = irc_user_on_destroy; + user->user_data = ctx; + user->nickname = nickname; + str_map_set (&ctx->irc_users, user->nickname, user); + return user; +} + +static void +irc_channel_on_destroy (void *object, void *user_data) +{ + struct channel *channel = object; + struct app_context *ctx = user_data; + str_map_set (&ctx->irc_channels, channel->name, NULL); +} + +struct channel * +irc_make_channel (struct app_context *ctx, char *name) +{ + hard_assert (!str_map_find (&ctx->irc_channels, name)); + + struct channel *channel = channel_new (); + channel->on_destroy = irc_channel_on_destroy; + channel->user_data = ctx; + channel->name = name; + channel->mode = xstrdup (""); + channel->topic = NULL; + str_map_set (&ctx->irc_channels, channel->name, channel); + return channel; +} + // --- Supporting code --------------------------------------------------------- static char * @@ -1833,11 +1880,7 @@ irc_handle_join (struct app_context *ctx, const struct irc_message *msg) // We've joined a new channel if (!channel && irc_is_this_us (ctx, msg->prefix)) { - channel = channel_new (); - channel->name = xstrdup (target); - channel->mode = xstrdup (""); - channel->topic = NULL; - str_map_set (&ctx->irc_channels, channel->name, channel); + channel = irc_make_channel (ctx, xstrdup (target)); buffer = buffer_new (); buffer->type = BUFFER_CHANNEL; @@ -1857,11 +1900,7 @@ irc_handle_join (struct app_context *ctx, const struct irc_message *msg) char *nickname = irc_cut_nickname (msg->prefix); struct user *user = str_map_find (&ctx->irc_users, nickname); if (!user) - { - user = user_new (); - user->nickname = nickname; - str_map_set (&ctx->irc_users, user->nickname, user); - } + user = irc_make_user (ctx, nickname); else free (nickname); @@ -2041,11 +2080,7 @@ irc_handle_privmsg (struct app_context *ctx, const struct irc_message *msg) char *nickname = irc_cut_nickname (msg->prefix); struct user *user = str_map_find (&ctx->irc_users, nickname); if (!user) - { - user = user_new (); - user->nickname = xstrdup (nickname); - str_map_set (&ctx->irc_users, user->nickname, user); - } + user = irc_make_user (ctx, xstrdup (nickname)); // Open a new buffer for the user buffer = buffer_new (); @@ -2120,15 +2155,6 @@ irc_handle_quit (struct app_context *ctx, const struct irc_message *msg) LIST_UNLINK (user->channels, iter); user_channel_destroy (iter); } - - // If it's the last reference, there's no reason for the user to stay. - // Note that when the user has their own buffer, it still keeps a reference - // and we don't have to care about removing them from "irc_buffer_map". - if (user->ref_count == 1) - { - hard_assert (!user->channels); - str_map_set (&ctx->irc_users, user->nickname, NULL); - } } static void @@ -2912,10 +2938,7 @@ irc_connect (struct app_context *ctx, struct error **e) irc_send (ctx, "USER %s 8 * :%s", username, realname); // XXX: maybe we should wait for the first message from the server - ctx->irc_user = user_new (); - ctx->irc_user->nickname = xstrdup (nickname); - str_map_set (&ctx->irc_users, nickname, user_ref (ctx->irc_user)); - + ctx->irc_user = irc_make_user (ctx, xstrdup (nickname)); ctx->irc_user_mode = xstrdup (""); ctx->irc_user_host = NULL; return true;