From 7caba06019ecc5775d3dbb513b70f52f620affb5 Mon Sep 17 00:00:00 2001 From: Christian Grothoff Date: Thu, 9 Aug 2018 16:35:32 +0200 Subject: [PATCH] change CADET channel destroy API to not call the callback if the client initiated the channel's destruction --- src/cadet/cadet_api.c | 3 +- .../gnunet-service-conversation.c | 74 ++++++++++++------- src/include/gnunet_cadet_service.h | 4 +- src/set/gnunet-service-set.c | 31 ++++++-- src/set/gnunet-service-set.h | 9 +++ src/set/gnunet-service-set_union.c | 23 +----- 6 files changed, 85 insertions(+), 59 deletions(-) diff --git a/src/cadet/cadet_api.c b/src/cadet/cadet_api.c index 85a8be522..980b9abbf 100644 --- a/src/cadet/cadet_api.c +++ b/src/cadet/cadet_api.c @@ -1273,7 +1273,7 @@ GNUNET_CADET_close_port (struct GNUNET_CADET_Port *p) /** * Destroy an existing channel. * - * The existing end callback for the channel will be called immediately. + * The existing end callback for the channel will NOT be called. * Any pending outgoing messages will be sent but no incoming messages will be * accepted and no data callbacks will be called. * @@ -1296,6 +1296,7 @@ GNUNET_CADET_channel_destroy (struct GNUNET_CADET_Channel *channel) } GNUNET_log (GNUNET_ERROR_TYPE_INFO, "Destroying channel due to GNUNET_CADET_channel_destroy()\n"); + channel->disconnects = NULL; destroy_channel (channel); } diff --git a/src/conversation/gnunet-service-conversation.c b/src/conversation/gnunet-service-conversation.c index fb9eec26e..059bb158b 100644 --- a/src/conversation/gnunet-service-conversation.c +++ b/src/conversation/gnunet-service-conversation.c @@ -302,6 +302,47 @@ handle_client_pickup_message (void *cls, } +/** + * Channel went down, notify client and free data + * structure. + * + * @param ch channel that went down + */ +static void +clean_up_channel (struct Channel *ch) +{ + struct Line *line = ch->line; + struct GNUNET_MQ_Envelope *env; + struct ClientPhoneHangupMessage *hup; + + switch (ch->status) + { + case CS_CALLEE_INIT: + case CS_CALLEE_SHUTDOWN: + case CS_CALLER_SHUTDOWN: + break; + case CS_CALLEE_RINGING: + case CS_CALLEE_CONNECTED: + case CS_CALLER_CALLING: + case CS_CALLER_CONNECTED: + if (NULL != line) + { + env = GNUNET_MQ_msg (hup, + GNUNET_MESSAGE_TYPE_CONVERSATION_CS_PHONE_HANG_UP); + hup->cid = ch->cid; + GNUNET_MQ_send (line->mq, + env); + } + break; + } + if (NULL != line) + GNUNET_CONTAINER_DLL_remove (line->channel_head, + line->channel_tail, + ch); + GNUNET_free (ch); +} + + /** * Destroy a channel. * @@ -313,7 +354,11 @@ destroy_line_cadet_channels (struct Channel *ch) GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Destroying cadet channels\n"); if (NULL != ch->channel) + { GNUNET_CADET_channel_destroy (ch->channel); + ch->channel = NULL; + } + clean_up_channel (ch); } @@ -1027,40 +1072,13 @@ inbound_end (void *cls, const struct GNUNET_CADET_Channel *channel) { struct Channel *ch = cls; - struct Line *line = ch->line; - struct GNUNET_MQ_Envelope *env; - struct ClientPhoneHangupMessage *hup; GNUNET_assert (channel == ch->channel); ch->channel = NULL; GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Channel destroyed by CADET in state %d\n", ch->status); - switch (ch->status) - { - case CS_CALLEE_INIT: - case CS_CALLEE_SHUTDOWN: - case CS_CALLER_SHUTDOWN: - break; - case CS_CALLEE_RINGING: - case CS_CALLEE_CONNECTED: - case CS_CALLER_CALLING: - case CS_CALLER_CONNECTED: - if (NULL != line) - { - env = GNUNET_MQ_msg (hup, - GNUNET_MESSAGE_TYPE_CONVERSATION_CS_PHONE_HANG_UP); - hup->cid = ch->cid; - GNUNET_MQ_send (line->mq, - env); - } - break; - } - if (NULL != line) - GNUNET_CONTAINER_DLL_remove (line->channel_head, - line->channel_tail, - ch); - GNUNET_free (ch); + clean_up_channel (ch); } diff --git a/src/include/gnunet_cadet_service.h b/src/include/gnunet_cadet_service.h index 552763055..276fe4dbc 100644 --- a/src/include/gnunet_cadet_service.h +++ b/src/include/gnunet_cadet_service.h @@ -151,7 +151,7 @@ typedef void * /** - * Function called whenever an MQ-channel is destroyed, even if the destruction + * Function called whenever an MQ-channel is destroyed, unless the destruction * was requested by #GNUNET_CADET_channel_destroy. * It must NOT call #GNUNET_CADET_channel_destroy on the channel. * @@ -277,7 +277,7 @@ GNUNET_CADET_channel_create (struct GNUNET_CADET_Handle *h, /** * Destroy an existing channel. * - * The existing end callback for the channel will be called immediately. + * The existing end callback for the channel will NOT be called. * Any pending outgoing messages will be sent but no incoming messages will be * accepted and no data callbacks will be called. * diff --git a/src/set/gnunet-service-set.c b/src/set/gnunet-service-set.c index 217b89d6d..75122395d 100644 --- a/src/set/gnunet-service-set.c +++ b/src/set/gnunet-service-set.c @@ -221,11 +221,7 @@ incoming_destroy (struct Operation *op) GNUNET_SCHEDULER_cancel (op->timeout_task); op->timeout_task = NULL; } - if (NULL != (channel = op->channel)) - { - op->channel = NULL; - GNUNET_CADET_channel_destroy (channel); - } + _GSS_operation_destroy2 (op); } @@ -1199,9 +1195,30 @@ channel_end_cb (void *channel_ctx, { struct Operation *op = channel_ctx; + op->channel = NULL; + _GSS_operation_destroy2 (op); +} + + +/** + * This function probably should not exist + * and be replaced by inlining more specific + * logic in the various places where it is called. + */ +void +_GSS_operation_destroy2 (struct Operation *op) +{ + struct GNUNET_CADET_Channel *channel; + GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "channel_end_cb called\n"); - op->channel = NULL; + if (NULL != (channel = op->channel)) + { + /* This will free op; called conditionally as this helper function + is also called from within the channel disconnect handler. */ + op->channel = NULL; + GNUNET_CADET_channel_destroy (channel); + } if (NULL != op->listener) incoming_destroy (op); else if (NULL != op->set) @@ -1376,7 +1393,7 @@ handle_client_reject (void *cls, "Peer request (op %u, app %s) rejected by client\n", op->listener->operation, GNUNET_h2s (&cs->listener->app_id)); - GNUNET_CADET_channel_destroy (op->channel); + _GSS_operation_destroy2 (op); GNUNET_SERVICE_client_continue (cs->client); } diff --git a/src/set/gnunet-service-set.h b/src/set/gnunet-service-set.h index f7c262eac..a58b22995 100644 --- a/src/set/gnunet-service-set.h +++ b/src/set/gnunet-service-set.h @@ -629,6 +629,15 @@ _GSS_operation_destroy (struct Operation *op, int gc); +/** + * This function probably should not exist + * and be replaced by inlining more specific + * logic in the various places where it is called. + */ +void +_GSS_operation_destroy2 (struct Operation *op); + + /** * Get the table with implementing functions for set union. * diff --git a/src/set/gnunet-service-set_union.c b/src/set/gnunet-service-set_union.c index c3c14f1ba..8c0c52d64 100644 --- a/src/set/gnunet-service-set_union.c +++ b/src/set/gnunet-service-set_union.c @@ -1366,25 +1366,6 @@ send_client_element (struct Operation *op, } -/** - * Destroy remote channel. - * - * @param op operation - */ -void destroy_channel (struct Operation *op) -{ - struct GNUNET_CADET_Channel *channel; - - if (NULL != (channel = op->channel)) - { - /* This will free op; called conditionally as this helper function - is also called from within the channel disconnect handler. */ - op->channel = NULL; - GNUNET_CADET_channel_destroy (channel); - } -} - - /** * Signal to the client that the operation has finished and * destroy the operation. @@ -1467,7 +1448,7 @@ maybe_finish (struct Operation *op) { op->state->phase = PHASE_DONE; send_client_done (op); - destroy_channel (op); + _GSS_operation_destroy2 (op); } } } @@ -1896,7 +1877,7 @@ handle_union_p2p_full_done (void *cls, op->state->phase = PHASE_DONE; GNUNET_CADET_receive_done (op->channel); send_client_done (op); - destroy_channel (op); + _GSS_operation_destroy2 (op); return; } break; -- 2.25.1