From caf6cccc5c75c310f0eedbd5b01b5d6751681e0d Mon Sep 17 00:00:00 2001 From: Joshua Colp Date: Tue, 7 Jun 2016 20:45:37 -0300 Subject: cel: Ensure only one dial status per channel exists. CEL wrongly assumed that a channel would only have a single dial event on it. This is incorrect. Particularly in a queue each call attempt to a member will result in a dial event, adding a new dial status in CEL without removing the old one. This would cause the container to grow with only one dial status being removed when the channel went away. The other dial status entries would remain leaking memory. This change fixes the memory leak by ensuring that only one dial status will only ever exist for each channel. The behavior during the scenario where multiple events are received has also been improved. For failure cases the first failure will be the dial status. If an answer dial status is received, though, it will take priority and the dial status for the channel will be answer. Memory usage has also been decreased by storing the minimal amount of information and the code has been cleaned up slightly. ASTERISK-25262 #close Change-Id: I5944eb923db17b6a0faa7317ff6abc9307c009fe --- main/cel.c | 108 ++++++++++++++++++++++++++++++++----------------------- tests/test_cel.c | 2 +- 2 files changed, 64 insertions(+), 46 deletions(-) diff --git a/main/cel.c b/main/cel.c index d9fcc5f6b..114e77a46 100644 --- a/main/cel.c +++ b/main/cel.c @@ -170,6 +170,13 @@ struct cel_linkedid { /*! Container of channel references to a linkedid for CEL purposes. */ static AO2_GLOBAL_OBJ_STATIC(cel_linkedids); +struct cel_dialstatus { + /*! Uniqueid of the channel */ + char uniqueid[AST_MAX_UNIQUEID]; + /*! The dial status */ + char dialstatus[0]; +}; + /*! \brief Destructor for cel_config */ static void cel_general_config_dtor(void *obj) { @@ -372,20 +379,10 @@ static int cel_backend_cmp(void *obj, void *arg, int flags) return CMP_MATCH; } -static const char *get_caller_uniqueid(struct ast_multi_channel_blob *blob) -{ - struct ast_channel_snapshot *caller = ast_multi_channel_blob_get_channel(blob, "caller"); - if (!caller) { - return NULL; - } - - return caller->uniqueid; -} - /*! \brief Hashing function for dialstatus container */ static int dialstatus_hash(const void *obj, int flags) { - struct ast_multi_channel_blob *blob; + const struct cel_dialstatus *dialstatus; const char *key; switch (flags & OBJ_SEARCH_MASK) { @@ -393,8 +390,8 @@ static int dialstatus_hash(const void *obj, int flags) key = obj; break; case OBJ_SEARCH_OBJECT: - blob = (void *) obj; - key = get_caller_uniqueid(blob); + dialstatus = obj; + key = dialstatus->uniqueid; break; default: /* Hash can only work on something with a full key. */ @@ -407,24 +404,24 @@ static int dialstatus_hash(const void *obj, int flags) /*! \brief Comparator function for dialstatus container */ static int dialstatus_cmp(void *obj, void *arg, int flags) { - struct ast_multi_channel_blob *object_left = obj; - struct ast_multi_channel_blob *object_right = arg; + struct cel_dialstatus *object_left = obj; + struct cel_dialstatus *object_right = arg; const char *right_key = arg; int cmp; switch (flags & OBJ_SEARCH_MASK) { case OBJ_SEARCH_OBJECT: - right_key = get_caller_uniqueid(object_right); + right_key = object_right->uniqueid; /* Fall through */ case OBJ_SEARCH_KEY: - cmp = strcmp(get_caller_uniqueid(object_left), right_key); + cmp = strcmp(object_left->uniqueid, right_key); break; case OBJ_SEARCH_PARTIAL_KEY: /* * We could also use a partial key struct containing a length * so strlen() does not get called for every comparison instead. */ - cmp = strncmp(get_caller_uniqueid(object_left), right_key, strlen(right_key)); + cmp = strncmp(object_left->uniqueid, right_key, strlen(right_key)); break; default: /* @@ -958,16 +955,16 @@ typedef void (*cel_channel_snapshot_monitor)( struct ast_channel_snapshot *old_snapshot, struct ast_channel_snapshot *new_snapshot); -static struct ast_multi_channel_blob *get_dialstatus_blob(const char *uniqueid) +static struct cel_dialstatus *get_dialstatus(const char *uniqueid) { struct ao2_container *dial_statuses = ao2_global_obj_ref(cel_dialstatus_store); - struct ast_multi_channel_blob *blob = NULL; + struct cel_dialstatus *dialstatus = NULL; if (dial_statuses) { - blob = ao2_find(dial_statuses, uniqueid, OBJ_SEARCH_KEY | OBJ_UNLINK); + dialstatus = ao2_find(dial_statuses, uniqueid, OBJ_SEARCH_KEY | OBJ_UNLINK); ao2_ref(dial_statuses, -1); } - return blob; + return dialstatus; } static const char *get_blob_variable(struct ast_multi_channel_blob *blob, const char *varname) @@ -1010,19 +1007,15 @@ static void cel_channel_state_change( if (!was_hungup && is_hungup) { struct ast_json *extra; - struct ast_multi_channel_blob *blob = get_dialstatus_blob(new_snapshot->uniqueid); - const char *dialstatus = ""; + struct cel_dialstatus *dialstatus = get_dialstatus(new_snapshot->uniqueid); - if (blob && !ast_strlen_zero(get_blob_variable(blob, "dialstatus"))) { - dialstatus = get_blob_variable(blob, "dialstatus"); - } extra = ast_json_pack("{s: i, s: s, s: s}", "hangupcause", new_snapshot->hangupcause, "hangupsource", new_snapshot->hangupsource, - "dialstatus", dialstatus); + "dialstatus", dialstatus ? dialstatus->dialstatus : ""); cel_report_event(new_snapshot, AST_CEL_HANGUP, NULL, extra, NULL); ast_json_unref(extra); - ao2_cleanup(blob); + ao2_cleanup(dialstatus); return; } @@ -1254,16 +1247,48 @@ static void cel_parking_cb( } } -static void save_dialstatus(struct ast_multi_channel_blob *blob) +static void save_dialstatus(struct ast_multi_channel_blob *blob, struct ast_channel_snapshot *snapshot) { struct ao2_container *dial_statuses = ao2_global_obj_ref(cel_dialstatus_store); + const char *dialstatus_string = get_blob_variable(blob, "dialstatus"); + struct cel_dialstatus *dialstatus; + size_t dialstatus_string_len; - ast_assert(blob != NULL); + if (!dial_statuses || ast_strlen_zero(dialstatus_string)) { + ao2_cleanup(dial_statuses); + return; + } - if (dial_statuses) { - ao2_link(dial_statuses, blob); + dialstatus = ao2_find(dial_statuses, snapshot->uniqueid, OBJ_SEARCH_KEY); + if (dialstatus) { + if (!strcasecmp(dialstatus_string, "ANSWER") && strcasecmp(dialstatus->dialstatus, "ANSWER")) { + /* In the case of an answer after we already have a dial status we give + * priority to the answer since the call was, well, answered. In the case of + * failure dial status results we simply let the first failure be the status. + */ + ao2_unlink(dial_statuses, dialstatus); + ao2_ref(dialstatus, -1); + } else { + ao2_ref(dialstatus, -1); + ao2_ref(dial_statuses, -1); + return; + } + } + + dialstatus_string_len = strlen(dialstatus_string) + 1; + dialstatus = ao2_alloc_options(sizeof(*dialstatus) + dialstatus_string_len, NULL, + AO2_ALLOC_OPT_LOCK_NOLOCK); + if (!dialstatus) { ao2_ref(dial_statuses, -1); + return; } + + ast_copy_string(dialstatus->uniqueid, snapshot->uniqueid, sizeof(dialstatus->uniqueid)); + ast_copy_string(dialstatus->dialstatus, dialstatus_string, dialstatus_string_len); + + ao2_link(dial_statuses, dialstatus); + ao2_ref(dialstatus, -1); + ao2_ref(dial_statuses, -1); } static int is_valid_dialstatus(struct ast_multi_channel_blob *blob) @@ -1299,32 +1324,25 @@ static void cel_dial_cb(void *data, struct stasis_subscription *sub, struct stasis_message *message) { struct ast_multi_channel_blob *blob = stasis_message_data(message); + struct ast_channel_snapshot *snapshot; - if (cel_filter_channel_snapshot(ast_multi_channel_blob_get_channel(blob, "caller"))) { - return; - } - - if (!get_caller_uniqueid(blob)) { + snapshot = ast_multi_channel_blob_get_channel(blob, "caller"); + if (!snapshot || cel_filter_channel_snapshot(snapshot)) { return; } if (!ast_strlen_zero(get_blob_variable(blob, "forward"))) { - struct ast_channel_snapshot *caller = ast_multi_channel_blob_get_channel(blob, "caller"); struct ast_json *extra; - if (!caller) { - return; - } - extra = ast_json_pack("{s: s}", "forward", get_blob_variable(blob, "forward")); if (extra) { - cel_report_event(caller, AST_CEL_FORWARD, NULL, extra, NULL); + cel_report_event(snapshot, AST_CEL_FORWARD, NULL, extra, NULL); ast_json_unref(extra); } } if (is_valid_dialstatus(blob)) { - save_dialstatus(blob); + save_dialstatus(blob, snapshot); } } diff --git a/tests/test_cel.c b/tests/test_cel.c index 03e243c78..9a3dc8114 100644 --- a/tests/test_cel.c +++ b/tests/test_cel.c @@ -1610,7 +1610,7 @@ AST_TEST_DEFINE(test_cel_dial_pickup) ast_channel_publish_dial(chan_caller, chan_callee, NULL, "ANSWER"); - HANGUP_CHANNEL(chan_caller, AST_CAUSE_NORMAL, "CANCEL"); + HANGUP_CHANNEL(chan_caller, AST_CAUSE_NORMAL, "ANSWER"); HANGUP_CHANNEL(chan_callee, AST_CAUSE_NORMAL, ""); return AST_TEST_PASS; -- cgit v1.2.3