[Asterisk-code-review] cel: Ensure only one dial status per channel exists. (asterisk[master])
Joshua Colp
asteriskteam at digium.com
Wed Jun 8 10:54:45 CDT 2016
Joshua Colp has uploaded a new change for review.
https://gerrit.asterisk.org/2973
Change subject: cel: Ensure only one dial status per channel exists.
......................................................................
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 improves the usage of dial status in CEL and also
fixes the memory leak.
The code will now ensure that only one dial status entry is
present in the container for a channel. The code also now adds
dial status information for peer (or callee) channels so that
the reason for their hangup will be conveyed in the CEL event
as well.
ASTERISK-25262 #close
Change-Id: I5944eb923db17b6a0faa7317ff6abc9307c009fe
---
M main/cel.c
1 file changed, 62 insertions(+), 54 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/73/2973/1
diff --git a/main/cel.c b/main/cel.c
index a0d0ad7..223f129 100644
--- a/main/cel.c
+++ b/main/cel.c
@@ -170,6 +170,13 @@
/*! 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 @@
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 @@
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 @@
/*! \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 @@
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 @@
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,34 @@
}
}
-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) {
- ao2_link(dial_statuses, blob);
- ao2_ref(dial_statuses, -1);
+ if (!dial_statuses || ast_strlen_zero(dialstatus_string)) {
+ ao2_cleanup(dial_statuses);
+ 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;
+ }
+
+ ao2_find(dial_statuses, snapshot->uniqueid, OBJ_NODATA | OBJ_SEARCH_KEY | OBJ_UNLINK);
+
+ 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 +1310,29 @@
struct stasis_message *message)
{
struct ast_multi_channel_blob *blob = stasis_message_data(message);
+ struct ast_channel_snapshot *snapshot;
+ int is_valid_status = is_valid_dialstatus(blob);
- if (cel_filter_channel_snapshot(ast_multi_channel_blob_get_channel(blob, "caller"))) {
- return;
- }
+ snapshot = ast_multi_channel_blob_get_channel(blob, "caller");
+ if (snapshot && !cel_filter_channel_snapshot(snapshot)) {
+ if (!ast_strlen_zero(get_blob_variable(blob, "forward"))) {
+ struct ast_json *extra;
- if (!get_caller_uniqueid(blob)) {
- 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(snapshot, AST_CEL_FORWARD, NULL, extra, NULL);
+ ast_json_unref(extra);
+ }
}
- extra = ast_json_pack("{s: s}", "forward", get_blob_variable(blob, "forward"));
- if (extra) {
- cel_report_event(caller, AST_CEL_FORWARD, NULL, extra, NULL);
- ast_json_unref(extra);
+ if (is_valid_status) {
+ save_dialstatus(blob, snapshot);
}
}
- if (is_valid_dialstatus(blob)) {
- save_dialstatus(blob);
+ snapshot = ast_multi_channel_blob_get_channel(blob, "peer");
+ if (snapshot && !cel_filter_channel_snapshot(snapshot) && is_valid_status) {
+ save_dialstatus(blob, snapshot);
}
}
--
To view, visit https://gerrit.asterisk.org/2973
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5944eb923db17b6a0faa7317ff6abc9307c009fe
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
More information about the asterisk-code-review
mailing list