[Asterisk-code-review] res stasis: Reduce RAII VAR usage. (asterisk[15])
Corey Farrell
asteriskteam at digium.com
Fri Jan 5 12:44:34 CST 2018
Corey Farrell has uploaded this change for review. ( https://gerrit.asterisk.org/7832
Change subject: res_stasis: Reduce RAII_VAR usage.
......................................................................
res_stasis: Reduce RAII_VAR usage.
In addition to being a micro-optimization (RAII_VAR has overhead), this
change improves output of REF_DEBUG. Unfortunately when RAII_VAR calls
ao2_cleanup it does so from a generated _dtor_varname function. For
example this caused _dtor_app to release a reference instead of
__stasis_app_unregister.
Change-Id: I4ce67120583a446babf9adeec678b71d37fcd9e5
---
M res/res_stasis.c
M res/stasis/app.c
M res/stasis/command.c
M res/stasis/control.c
M res/stasis/stasis_bridge.c
5 files changed, 297 insertions(+), 182 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/32/7832/1
diff --git a/res/res_stasis.c b/res/res_stasis.c
index 42a19bf..a325336 100644
--- a/res/res_stasis.c
+++ b/res/res_stasis.c
@@ -498,7 +498,8 @@
/*! Request a bridge MOH channel */
static struct ast_channel *prepare_bridge_moh_channel(void)
{
- RAII_VAR(struct ast_format_cap *, cap, NULL, ao2_cleanup);
+ struct ast_channel *chan;
+ struct ast_format_cap *cap;
cap = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
if (!cap) {
@@ -507,7 +508,10 @@
ast_format_cap_append(cap, ast_format_slin, 0);
- return ast_request("Announcer", cap, NULL, NULL, "ARI_MOH", NULL);
+ chan = ast_request("Announcer", cap, NULL, NULL, "ARI_MOH", NULL);
+ ao2_ref(cap, -1);
+
+ return chan;
}
/*! Provides the moh channel with a thread so it can actually play its music */
@@ -599,23 +603,27 @@
struct ast_channel *stasis_app_bridge_moh_channel(struct ast_bridge *bridge)
{
- RAII_VAR(struct stasis_app_bridge_channel_wrapper *, moh_wrapper, NULL, ao2_cleanup);
+ struct ast_channel *chan;
+ struct stasis_app_bridge_channel_wrapper *moh_wrapper;
- {
- SCOPED_AO2LOCK(lock, app_bridges_moh);
+ ao2_lock(app_bridges_moh);
+ moh_wrapper = ao2_find(app_bridges_moh, bridge->uniqueid, OBJ_SEARCH_KEY | OBJ_NOLOCK);
+ if (!moh_wrapper) {
+ chan = bridge_moh_create(bridge);
+ }
+ ao2_unlock(app_bridges_moh);
- moh_wrapper = ao2_find(app_bridges_moh, bridge->uniqueid, OBJ_SEARCH_KEY | OBJ_NOLOCK);
- if (!moh_wrapper) {
- return bridge_moh_create(bridge);
- }
+ if (moh_wrapper) {
+ chan = ast_channel_get_by_name(moh_wrapper->channel_id);
+ ao2_ref(moh_wrapper, -1);
}
- return ast_channel_get_by_name(moh_wrapper->channel_id);
+ return chan;
}
int stasis_app_bridge_moh_stop(struct ast_bridge *bridge)
{
- RAII_VAR(struct stasis_app_bridge_channel_wrapper *, moh_wrapper, NULL, ao2_cleanup);
+ struct stasis_app_bridge_channel_wrapper *moh_wrapper;
struct ast_channel *chan;
moh_wrapper = ao2_find(app_bridges_moh, bridge->uniqueid, OBJ_SEARCH_KEY | OBJ_UNLINK);
@@ -624,6 +632,7 @@
}
chan = ast_channel_get_by_name(moh_wrapper->channel_id);
+ ao2_ref(moh_wrapper, -1);
if (!chan) {
return -1;
}
@@ -862,25 +871,30 @@
static struct replace_channel_store *get_replace_channel_store(struct ast_channel *chan, int no_create)
{
struct ast_datastore *datastore;
+ struct replace_channel_store *ret;
- SCOPED_CHANNELLOCK(lock, chan);
+ ast_channel_lock(chan);
datastore = ast_channel_datastore_find(chan, &replace_channel_store_info, NULL);
- if (!datastore) {
- if (no_create) {
- return NULL;
- }
-
+ if (!datastore && !no_create) {
datastore = ast_datastore_alloc(&replace_channel_store_info, NULL);
- if (!datastore) {
- return NULL;
+ if (datastore) {
+ ast_channel_datastore_add(chan, datastore);
}
- ast_channel_datastore_add(chan, datastore);
+ }
+
+ if (!datastore) {
+ ast_channel_unlock(chan);
+ return NULL;
}
if (!datastore->data) {
datastore->data = ast_calloc(1, sizeof(struct replace_channel_store));
}
- return datastore->data;
+
+ ret = datastore->data;
+ ast_channel_unlock(chan);
+
+ return ret;
}
int app_set_replace_channel_snapshot(struct ast_channel *chan, struct ast_channel_snapshot *replace_snapshot)
@@ -959,9 +973,9 @@
int argc, char *argv[], struct ast_channel_snapshot *snapshot,
struct ast_channel_snapshot *replace_channel_snapshot)
{
- RAII_VAR(struct ast_json *, json_blob, NULL, ast_json_unref);
+ struct ast_json *json_blob;
struct ast_json *json_args;
- RAII_VAR(struct start_message_blob *, payload, NULL, ao2_cleanup);
+ struct start_message_blob *payload;
struct stasis_message *msg;
int i;
@@ -986,8 +1000,11 @@
"args");
if (!json_blob) {
ast_log(LOG_ERROR, "Error packing JSON for StasisStart message\n");
+ ao2_ref(payload, -1);
return -1;
}
+ payload->blob = json_blob;
+
/* Append arguments to args array */
json_args = ast_json_object_get(json_blob, "args");
@@ -997,13 +1014,14 @@
ast_json_string_create(argv[i]));
if (r != 0) {
ast_log(LOG_ERROR, "Error appending to StasisStart message\n");
+ ao2_ref(payload, -1);
return -1;
}
}
- payload->blob = ast_json_ref(json_blob);
msg = stasis_message_create(start_message_type(), payload);
+ ao2_ref(payload, -1);
if (!msg) {
ast_log(LOG_ERROR, "Error sending StasisStart message\n");
return -1;
@@ -1020,9 +1038,9 @@
static int send_start_msg(struct stasis_app *app, struct ast_channel *chan,
int argc, char *argv[])
{
- RAII_VAR(struct ast_channel_snapshot *, snapshot, NULL, ao2_cleanup);
- RAII_VAR(struct ast_channel_snapshot *, replace_channel_snapshot,
- NULL, ao2_cleanup);
+ int ret = -1;
+ struct ast_channel_snapshot *snapshot;
+ struct ast_channel_snapshot *replace_channel_snapshot;
ast_assert(chan != NULL);
@@ -1032,10 +1050,13 @@
ast_channel_lock(chan);
snapshot = ast_channel_snapshot_create(chan);
ast_channel_unlock(chan);
- if (!snapshot) {
- return -1;
+ if (snapshot) {
+ ret = send_start_msg_snapshots(chan, app, argc, argv, snapshot, replace_channel_snapshot);
+ ao2_ref(snapshot, -1);
}
- return send_start_msg_snapshots(chan, app, argc, argv, snapshot, replace_channel_snapshot);
+ ao2_cleanup(replace_channel_snapshot);
+
+ return ret;
}
static void remove_masquerade_store(struct ast_channel *chan);
@@ -1478,7 +1499,7 @@
int stasis_app_send(const char *app_name, struct ast_json *message)
{
- RAII_VAR(struct stasis_app *, app, NULL, ao2_cleanup);
+ struct stasis_app *app;
if (!apps_registry) {
return -1;
@@ -1494,6 +1515,8 @@
return -1;
}
app_send(app, message);
+ ao2_ref(app, -1);
+
return 0;
}
@@ -1528,7 +1551,7 @@
struct ao2_container *stasis_app_get_all(void)
{
- RAII_VAR(struct ao2_container *, apps, NULL, ao2_cleanup);
+ struct ao2_container *apps;
if (!apps_registry) {
return NULL;
@@ -1541,12 +1564,12 @@
ao2_callback(apps_registry, OBJ_NODATA, append_name, apps);
- return ao2_bump(apps);
+ return apps;
}
static int __stasis_app_register(const char *app_name, stasis_app_cb handler, void *data, int all_events)
{
- RAII_VAR(struct stasis_app *, app, NULL, ao2_cleanup);
+ struct stasis_app *app;
if (!apps_registry) {
return -1;
@@ -1558,24 +1581,25 @@
app_update(app, handler, data);
} else {
app = app_create(app_name, handler, data, all_events ? STASIS_APP_SUBSCRIBE_ALL : STASIS_APP_SUBSCRIBE_MANUAL);
- if (app) {
- if (all_events) {
- struct stasis_app_event_source *source;
- SCOPED_LOCK(lock, &event_sources, AST_RWLIST_RDLOCK, AST_RWLIST_UNLOCK);
-
- AST_LIST_TRAVERSE(&event_sources, source, next) {
- if (!source->subscribe) {
- continue;
- }
-
- source->subscribe(app, NULL);
- }
- }
- ao2_link_flags(apps_registry, app, OBJ_NOLOCK);
- } else {
+ if (!app) {
ao2_unlock(apps_registry);
return -1;
}
+
+ if (all_events) {
+ struct stasis_app_event_source *source;
+
+ AST_RWLIST_RDLOCK(&event_sources);
+ AST_LIST_TRAVERSE(&event_sources, source, next) {
+ if (!source->subscribe) {
+ continue;
+ }
+
+ source->subscribe(app, NULL);
+ }
+ AST_RWLIST_UNLOCK(&event_sources);
+ }
+ ao2_link_flags(apps_registry, app, OBJ_NOLOCK);
}
/* We lazily clean up the apps_registry, because it's good enough to
@@ -1583,6 +1607,7 @@
*/
cleanup();
ao2_unlock(apps_registry);
+ ao2_ref(app, -1);
return 0;
}
@@ -1598,7 +1623,7 @@
void stasis_app_unregister(const char *app_name)
{
- RAII_VAR(struct stasis_app *, app, NULL, ao2_cleanup);
+ struct stasis_app *app;
if (!app_name) {
return;
@@ -1616,6 +1641,7 @@
}
app_deactivate(app);
+ ao2_ref(app, -1);
/* There's a decent chance that app is ready for cleanup. Go ahead
* and clean up, just in case
@@ -1625,19 +1651,21 @@
void stasis_app_register_event_source(struct stasis_app_event_source *obj)
{
- SCOPED_LOCK(lock, &event_sources, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
+ AST_RWLIST_RDLOCK(&event_sources);
AST_LIST_INSERT_TAIL(&event_sources, obj, next);
/* only need to bump the module ref on non-core sources because the
core ones are [un]registered by this module. */
if (!stasis_app_is_core_event_source(obj)) {
ast_module_ref(ast_module_info->self);
}
+ AST_RWLIST_UNLOCK(&event_sources);
}
void stasis_app_unregister_event_source(struct stasis_app_event_source *obj)
{
struct stasis_app_event_source *source;
- SCOPED_LOCK(lock, &event_sources, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
+
+ AST_RWLIST_WRLOCK(&event_sources);
AST_RWLIST_TRAVERSE_SAFE_BEGIN(&event_sources, source, next) {
if (source == obj) {
AST_RWLIST_REMOVE_CURRENT(next);
@@ -1648,6 +1676,7 @@
}
}
AST_RWLIST_TRAVERSE_SAFE_END;
+ AST_RWLIST_UNLOCK(&event_sources);
}
/*!
@@ -1666,12 +1695,16 @@
const struct stasis_app *app, struct ast_json *json)
{
struct stasis_app_event_source *source;
- SCOPED_LOCK(lock, &event_sources, AST_RWLIST_RDLOCK, AST_RWLIST_UNLOCK);
+
+ AST_RWLIST_RDLOCK(&event_sources);
AST_LIST_TRAVERSE(&event_sources, source, next) {
if (source->to_json) {
source->to_json(app, json);
}
}
+ AST_RWLIST_UNLOCK(&event_sources);
+
+
return json;
}
@@ -1686,9 +1719,12 @@
struct ast_json *stasis_app_to_json(const char *app_name)
{
- RAII_VAR(struct stasis_app *, app, find_app_by_name(app_name), ao2_cleanup);
+ struct stasis_app *app = find_app_by_name(app_name);
+ struct ast_json *json = stasis_app_object_to_json(app);
- return stasis_app_object_to_json(app);
+ ao2_cleanup(app);
+
+ return json;
}
/*!
@@ -1705,13 +1741,16 @@
static struct stasis_app_event_source *app_event_source_find(const char *uri)
{
struct stasis_app_event_source *source;
- SCOPED_LOCK(lock, &event_sources, AST_RWLIST_RDLOCK, AST_RWLIST_UNLOCK);
+
+ AST_RWLIST_RDLOCK(&event_sources);
AST_LIST_TRAVERSE(&event_sources, source, next) {
if (ast_begins_with(uri, source->scheme)) {
- return source;
+ break;
}
}
- return NULL;
+ AST_RWLIST_UNLOCK(&event_sources);
+
+ return source;
}
/*!
@@ -1746,8 +1785,9 @@
int event_sources_count, struct ast_json **json,
app_subscription_handler handler)
{
- RAII_VAR(struct stasis_app *, app, find_app_by_name(app_name), ao2_cleanup);
+ struct stasis_app *app = find_app_by_name(app_name);
int i;
+ enum stasis_app_subscribe_res res = STASIS_ASR_OK;
if (!app) {
return STASIS_ASR_APP_NOT_FOUND;
@@ -1755,32 +1795,34 @@
for (i = 0; i < event_sources_count; ++i) {
const char *uri = event_source_uris[i];
- enum stasis_app_subscribe_res res = STASIS_ASR_INTERNAL_ERROR;
struct stasis_app_event_source *event_source;
if (!(event_source = app_event_source_find(uri))) {
ast_log(LOG_WARNING, "Invalid scheme: %s\n", uri);
+ ao2_ref(app, -1);
+
return STASIS_ASR_EVENT_SOURCE_BAD_SCHEME;
}
- if (handler &&
- ((res = handler(app, uri, event_source)))) {
- return res;
+ if (handler) {
+ res = handler(app, uri, event_source);
}
}
- if (json) {
+ if (res == STASIS_ASR_OK && json) {
ast_debug(3, "%s: Successful; setting results\n", app_name);
*json = stasis_app_object_to_json(app);
}
- return STASIS_ASR_OK;
+ ao2_ref(app, -1);
+
+ return res;
}
enum stasis_app_subscribe_res stasis_app_subscribe_channel(const char *app_name,
struct ast_channel *chan)
{
- RAII_VAR(struct stasis_app *, app, find_app_by_name(app_name), ao2_cleanup);
+ struct stasis_app *app = find_app_by_name(app_name);
int res;
if (!app) {
@@ -1790,6 +1832,8 @@
ast_debug(3, "%s: Subscribing to %s\n", app_name, ast_channel_uniqueid(chan));
res = app_subscribe_channel(app, chan);
+ ao2_ref(app, -1);
+
if (res != 0) {
ast_log(LOG_ERROR, "Error subscribing app '%s' to channel '%s'\n",
app_name, ast_channel_uniqueid(chan));
@@ -1892,12 +1936,10 @@
struct ast_json *json_variables)
{
RAII_VAR(struct stasis_app *, app, find_app_by_name(app_name), ao2_cleanup);
- RAII_VAR(struct ast_json *, blob, NULL, ast_json_unref);
- RAII_VAR(struct ast_multi_object_blob *, multi, NULL, ao2_cleanup);
- RAII_VAR(void *, obj, NULL, ao2_cleanup);
- RAII_VAR(struct stasis_message *, message, NULL, ao2_cleanup);
+ struct ast_json *blob = NULL;
+ struct ast_multi_object_blob *multi;
+ struct stasis_message *message;
enum stasis_app_user_event_res res = STASIS_APP_USER_INTERNAL_ERROR;
- struct ast_json *json_value;
int have_channel = 0;
int i;
@@ -1910,23 +1952,24 @@
return res;
}
- blob = json_variables;
- if (!blob) {
- blob = ast_json_pack("{}");
+ if (json_variables) {
+ struct ast_json *json_value = ast_json_string_create(event_name);
+
+ if (json_value && !ast_json_object_set(blob, "eventname", json_value)) {
+ blob = ast_json_ref(json_variables);
+ }
} else {
- ast_json_ref(blob);
+ blob = ast_json_pack("{ss}", "eventname", event_name);
}
- json_value = ast_json_string_create(event_name);
- if (!json_value) {
- ast_log(LOG_ERROR, "unable to create json string\n");
- return res;
- }
- if (ast_json_object_set(blob, "eventname", json_value)) {
- ast_log(LOG_ERROR, "unable to set eventname to blob\n");
+
+ if (!blob) {
+ ast_log(LOG_ERROR, "Failed to initialize blob\n");
+
return res;
}
multi = ast_multi_object_blob_create(blob);
+ ast_json_unref(blob);
for (i = 0; i < sources_count; ++i) {
const char *uri = source_uris[i];
@@ -1945,16 +1988,22 @@
snapshot = ast_endpoint_latest_snapshot(uri + 9, NULL);
} else {
ast_log(LOG_WARNING, "Invalid scheme: %s\n", uri);
+ ao2_ref(multi, -1);
+
return STASIS_APP_USER_EVENT_SOURCE_BAD_SCHEME;
}
if (!snapshot) {
ast_log(LOG_ERROR, "Unable to get snapshot for %s\n", uri);
+ ao2_ref(multi, -1);
+
return STASIS_APP_USER_EVENT_SOURCE_NOT_FOUND;
}
ast_multi_object_blob_add(multi, type, snapshot);
}
message = stasis_message_create(ast_multi_user_event_type(), multi);
+ ao2_ref(multi, -1);
+
if (!message) {
ast_log(LOG_ERROR, "Unable to create stasis user event message\n");
return res;
@@ -1971,6 +2020,7 @@
if (have_channel) {
stasis_publish(ast_manager_get_topic(), message);
}
+ ao2_ref(message, -1);
return STASIS_APP_USER_OK;
}
@@ -2036,9 +2086,14 @@
/* \brief Sanitization callback for channel unique IDs */
static int channel_id_sanitizer(const char *id)
{
- RAII_VAR(struct ast_channel_snapshot *, snapshot, ast_channel_snapshot_get_latest(id), ao2_cleanup);
+ struct ast_channel_snapshot *snapshot;
+ int ret;
- return channel_snapshot_sanitizer(snapshot);
+ snapshot = ast_channel_snapshot_get_latest(id);
+ ret = channel_snapshot_sanitizer(snapshot);
+ ao2_cleanup(snapshot);
+
+ return ret;
}
/* \brief Sanitization callbacks for communication to Stasis applications */
diff --git a/res/stasis/app.c b/res/stasis/app.c
index 667636e..731114a 100644
--- a/res/stasis/app.c
+++ b/res/stasis/app.c
@@ -112,20 +112,19 @@
static struct app_forwards *forwards_create(struct stasis_app *app,
const char *id)
{
- RAII_VAR(struct app_forwards *, forwards, NULL, ao2_cleanup);
+ struct app_forwards *forwards;
if (!app || ast_strlen_zero(id)) {
return NULL;
}
- forwards = ao2_alloc(sizeof(*forwards) + strlen(id) + 1, forwards_dtor);
+ forwards = ao2_t_alloc(sizeof(*forwards) + strlen(id) + 1, forwards_dtor, id);
if (!forwards) {
return NULL;
}
- strcpy(forwards->id, id);
+ strcpy(forwards->id, id); /* SAFE */
- ao2_ref(forwards, +1);
return forwards;
}
@@ -335,7 +334,7 @@
struct stasis_message *message)
{
struct stasis_app *app = data;
- RAII_VAR(struct ast_json *, json, NULL, ast_json_unref);
+ struct ast_json *json;
if (stasis_subscription_final_message(sub, message)) {
ao2_cleanup(app);
@@ -352,6 +351,7 @@
}
app_send(app, json);
+ ast_json_unref(json);
}
/*! \brief Typedef for callbacks that get called on channel snapshot updates */
@@ -554,11 +554,12 @@
stasis_message_timestamp(message);
for (i = 0; i < ARRAY_LEN(channel_monitors); ++i) {
- RAII_VAR(struct ast_json *, msg, NULL, ast_json_unref);
+ struct ast_json *msg;
msg = channel_monitors[i](old_snapshot, new_snapshot, tv);
if (msg) {
app_send(app, msg);
+ ast_json_unref(msg);
}
}
@@ -586,7 +587,7 @@
static int message_received_handler(const char *endpoint_id, struct ast_json *json_msg, void *pvt)
{
- RAII_VAR(struct ast_endpoint_snapshot *, snapshot, NULL, ao2_cleanup);
+ struct ast_endpoint_snapshot *snapshot;
struct ast_json *json_endpoint;
struct ast_json *message;
struct stasis_app *app = pvt;
@@ -610,6 +611,7 @@
}
json_endpoint = ast_endpoint_snapshot_to_json(snapshot, stasis_app_get_sanitizer());
+ ao2_ref(snapshot, -1);
if (!json_endpoint) {
return -1;
}
@@ -631,7 +633,6 @@
struct stasis_subscription *sub,
struct stasis_message *message)
{
- RAII_VAR(struct ast_json *, json, NULL, ast_json_unref);
struct stasis_app *app = data;
struct stasis_cache_update *update;
struct ast_endpoint_snapshot *new_snapshot;
@@ -648,6 +649,8 @@
old_snapshot = stasis_message_data(update->old_snapshot);
if (new_snapshot) {
+ struct ast_json *json;
+
tv = stasis_message_timestamp(update->new_snapshot);
json = simple_endpoint_event("EndpointStateChange", new_snapshot, tv);
@@ -656,6 +659,7 @@
}
app_send(app, json);
+ ast_json_unref(json);
}
if (!new_snapshot && old_snapshot) {
@@ -683,7 +687,7 @@
struct stasis_subscription *sub,
struct stasis_message *message)
{
- RAII_VAR(struct ast_json *, json, NULL, ast_json_unref);
+ struct ast_json *json = NULL;
struct stasis_app *app = data;
struct stasis_cache_update *update;
struct ast_bridge_snapshot *new_snapshot;
@@ -717,6 +721,7 @@
if (json) {
app_send(app, json);
+ ast_json_unref(json);
}
if (!new_snapshot && old_snapshot) {
@@ -923,7 +928,7 @@
struct stasis_app *app_create(const char *name, stasis_app_cb handler, void *data, enum stasis_app_subscription_model subscription_model)
{
- RAII_VAR(struct stasis_app *, app, NULL, ao2_cleanup);
+ struct stasis_app *app;
size_t size;
int res = 0;
@@ -942,17 +947,12 @@
app->forwards = ao2_container_alloc_rbtree(AO2_ALLOC_OPT_LOCK_MUTEX,
AO2_CONTAINER_ALLOC_OPT_DUPS_OBJ_REJECT,
forwards_sort, NULL);
- if (!app->forwards) {
- return NULL;
- }
-
app->topic = stasis_topic_create(name);
- if (!app->topic) {
- return NULL;
- }
-
app->bridge_router = stasis_message_router_create(ast_bridge_topic_all());
- if (!app->bridge_router) {
+
+ if (!app->forwards || !app->topic || !app->bridge_router) {
+ ao2_ref(app, -1);
+
return NULL;
}
@@ -969,13 +969,17 @@
bridge_default_handler, app);
if (res != 0) {
+ ao2_ref(app, -1);
+
return NULL;
}
/* Bridge router holds a reference */
- ao2_ref(app, +1);
+ ao2_t_ref(app, +1, "app->bridge_router");
app->router = stasis_message_router_create(app->topic);
if (!app->router) {
+ ao2_ref(app, -1);
+
return NULL;
}
@@ -992,16 +996,17 @@
sub_default_handler, app);
if (res != 0) {
+ ao2_ref(app, -1);
+
return NULL;
}
/* Router holds a reference */
- ao2_ref(app, +1);
+ ao2_t_ref(app, +1, "app->router");
strncpy(app->name, name, size - sizeof(*app));
app->handler = handler;
app->data = ao2_bump(data);
- ao2_ref(app, +1);
return app;
}
@@ -1019,7 +1024,7 @@
{
stasis_app_cb handler;
char eid[20];
- RAII_VAR(void *, data, NULL, ao2_cleanup);
+ void *data = NULL;
if (ast_json_object_set(message, "asterisk_id", ast_json_string_create(
ast_eid_to_str(eid, sizeof(eid), &ast_eid_default)))) {
@@ -1028,37 +1033,40 @@
}
/* Copy off mutable state with lock held */
- {
- SCOPED_AO2LOCK(lock, app);
- handler = app->handler;
- if (app->data) {
- ao2_ref(app->data, +1);
- data = app->data;
- }
- /* Name is immutable; no need to copy */
+ ao2_lock(app);
+ handler = app->handler;
+ if (app->data) {
+ ao2_ref(app->data, +1);
+ data = app->data;
}
+ ao2_unlock(app);
+ /* Name is immutable; no need to copy */
- if (!handler) {
+ if (handler) {
+ handler(data, app->name, message);
+ } else {
ast_verb(3,
"Inactive Stasis app '%s' missed message\n", app->name);
- return;
}
+ ao2_ref(data, -1);
- handler(data, app->name, message);
}
void app_deactivate(struct stasis_app *app)
{
- SCOPED_AO2LOCK(lock, app);
+ ao2_lock(app);
+
ast_verb(1, "Deactivating Stasis app '%s'\n", app->name);
app->handler = NULL;
ao2_cleanup(app->data);
app->data = NULL;
+
+ ao2_unlock(app);
}
void app_shutdown(struct stasis_app *app)
{
- SCOPED_AO2LOCK(lock, app);
+ ao2_lock(app);
ast_assert(app_is_finished(app));
@@ -1068,27 +1076,37 @@
app->bridge_router = NULL;
stasis_message_router_unsubscribe(app->endpoint_router);
app->endpoint_router = NULL;
+
+ ao2_unlock(app);
}
int app_is_active(struct stasis_app *app)
{
- SCOPED_AO2LOCK(lock, app);
- return app->handler != NULL;
+ int ret;
+
+ ao2_lock(app);
+ ret = app->handler != NULL;
+ ao2_unlock(app);
+
+ return ret;
}
int app_is_finished(struct stasis_app *app)
{
- SCOPED_AO2LOCK(lock, app);
+ int ret;
- return app->handler == NULL && ao2_container_count(app->forwards) == 0;
+ ao2_lock(app);
+ ret = app->handler == NULL && ao2_container_count(app->forwards) == 0;
+ ao2_unlock(app);
+
+ return ret;
}
void app_update(struct stasis_app *app, stasis_app_cb handler, void *data)
{
- SCOPED_AO2LOCK(lock, app);
-
+ ao2_lock(app);
if (app->handler && app->data) {
- RAII_VAR(struct ast_json *, msg, NULL, ast_json_unref);
+ struct ast_json *msg;
ast_verb(1, "Replacing Stasis app '%s'\n", app->name);
@@ -1097,6 +1115,7 @@
"application", app->name);
if (msg) {
app_send(app, msg);
+ ast_json_unref(msg);
}
} else {
ast_verb(1, "Activating Stasis app '%s'\n", app->name);
@@ -1108,6 +1127,7 @@
ao2_ref(data, +1);
}
app->data = data;
+ ao2_unlock(app);
}
const char *stasis_app_name(const struct stasis_app *app)
@@ -1184,52 +1204,59 @@
struct ast_json *app_to_json(const struct stasis_app *app)
{
- RAII_VAR(struct ast_json *, json, NULL, ast_json_unref);
+ struct ast_json *json;
struct ast_json *channels;
struct ast_json *bridges;
struct ast_json *endpoints;
struct ao2_iterator i;
- void *obj;
+ struct app_forwards *forwards;
json = ast_json_pack("{s: s, s: [], s: [], s: []}",
"name", app->name,
"channel_ids", "bridge_ids", "endpoint_ids");
+ if (!json) {
+ return NULL;
+ }
channels = ast_json_object_get(json, "channel_ids");
bridges = ast_json_object_get(json, "bridge_ids");
endpoints = ast_json_object_get(json, "endpoint_ids");
+ if (!channels || !bridges || !endpoints) {
+ ast_json_unref(json);
+
+ return NULL;
+ }
i = ao2_iterator_init(app->forwards, 0);
- while ((obj = ao2_iterator_next(&i))) {
- RAII_VAR(struct app_forwards *, forwards, obj, ao2_cleanup);
- RAII_VAR(struct ast_json *, id, NULL, ast_json_unref);
+ while ((forwards = ao2_iterator_next(&i))) {
+ struct ast_json *id;
int append_res = -1;
id = ast_json_string_create(forwards->id);
switch (forwards->forward_type) {
case FORWARD_CHANNEL:
- append_res = ast_json_array_append(channels,
- ast_json_ref(id));
+ append_res = ast_json_array_append(channels, id);
break;
case FORWARD_BRIDGE:
- append_res = ast_json_array_append(bridges,
- ast_json_ref(id));
+ append_res = ast_json_array_append(bridges, id);
break;
case FORWARD_ENDPOINT:
- append_res = ast_json_array_append(endpoints,
- ast_json_ref(id));
+ append_res = ast_json_array_append(endpoints, id);
break;
}
+ ao2_ref(forwards, -1);
if (append_res != 0) {
ast_log(LOG_ERROR, "Error building response\n");
ao2_iterator_destroy(&i);
+ ast_json_unref(json);
+
return NULL;
}
}
ao2_iterator_destroy(&i);
- return ast_json_ref(json);
+ return json;
}
int app_subscribe_channel(struct stasis_app *app, struct ast_channel *chan)
@@ -1284,8 +1311,7 @@
static int unsubscribe(struct stasis_app *app, const char *kind, const char *id, int terminate)
{
- RAII_VAR(struct app_forwards *, forwards, NULL, ao2_cleanup);
- SCOPED_AO2LOCK(lock, app->forwards);
+ struct app_forwards *forwards;
if (!id) {
if (!strcmp(kind, "bridge")) {
@@ -1300,6 +1326,7 @@
}
}
+ ao2_lock(app->forwards);
forwards = ao2_find(app->forwards, id, OBJ_SEARCH_KEY | OBJ_NOLOCK);
if (!forwards) {
ast_debug(3, "App '%s' not subscribed to %s '%s'\n", app->name, kind, id);
@@ -1320,6 +1347,8 @@
messaging_app_unsubscribe_endpoint(app->name, id);
}
}
+ ao2_unlock(app->forwards);
+ ao2_ref(forwards, -1);
return 0;
}
@@ -1344,12 +1373,14 @@
int app_is_subscribed_channel_id(struct stasis_app *app, const char *channel_id)
{
- RAII_VAR(struct app_forwards *, forwards, NULL, ao2_cleanup);
+ struct app_forwards *forwards;
if (ast_strlen_zero(channel_id)) {
channel_id = CHANNEL_ALL;
}
forwards = ao2_find(app->forwards, channel_id, OBJ_SEARCH_KEY);
+ ao2_cleanup(forwards);
+
return forwards != NULL;
}
@@ -1369,16 +1400,18 @@
int app_subscribe_bridge(struct stasis_app *app, struct ast_bridge *bridge)
{
struct app_forwards *forwards;
- SCOPED_AO2LOCK(lock, app->forwards);
if (!app) {
return -1;
}
+ ao2_lock(app->forwards);
/* If subscribed to all, don't subscribe again */
forwards = ao2_find(app->forwards, BRIDGE_ALL, OBJ_SEARCH_KEY | OBJ_NOLOCK);
if (forwards) {
+ ao2_unlock(app->forwards);
ao2_ref(forwards, -1);
+
return 0;
}
@@ -1388,10 +1421,13 @@
/* Forwards not found, create one */
forwards = forwards_create_bridge(app, bridge);
if (!forwards) {
+ ao2_unlock(app->forwards);
+
return -1;
}
ao2_link_flags(app->forwards, forwards, OBJ_NOLOCK);
}
+ ao2_unlock(app->forwards);
++forwards->interested;
ast_debug(3, "Bridge '%s' is %d interested in %s\n",
@@ -1400,6 +1436,7 @@
app->name);
ao2_ref(forwards, -1);
+
return 0;
}
@@ -1429,25 +1466,15 @@
int app_is_subscribed_bridge_id(struct stasis_app *app, const char *bridge_id)
{
struct app_forwards *forwards;
- SCOPED_AO2LOCK(lock, app->forwards);
-
- forwards = ao2_find(app->forwards, BRIDGE_ALL, OBJ_SEARCH_KEY | OBJ_NOLOCK);
- if (forwards) {
- ao2_ref(forwards, -1);
- return 1;
- }
if (ast_strlen_zero(bridge_id)) {
bridge_id = BRIDGE_ALL;
}
- forwards = ao2_find(app->forwards, bridge_id, OBJ_SEARCH_KEY | OBJ_NOLOCK);
- if (forwards) {
- ao2_ref(forwards, -1);
- return 1;
- }
+ forwards = ao2_find(app->forwards, bridge_id, OBJ_SEARCH_KEY);
+ ao2_cleanup(forwards);
- return 0;
+ return forwards != NULL;
}
static void *bridge_find(const struct stasis_app *app, const char *id)
@@ -1472,10 +1499,13 @@
return -1;
}
+ ao2_lock(app->forwards);
/* If subscribed to all, don't subscribe again */
forwards = ao2_find(app->forwards, ENDPOINT_ALL, OBJ_SEARCH_KEY | OBJ_NOLOCK);
if (forwards) {
+ ao2_unlock(app->forwards);
ao2_ref(forwards, -1);
+
return 0;
}
@@ -1486,6 +1516,8 @@
/* Forwards not found, create one */
forwards = forwards_create_endpoint(app, endpoint);
if (!forwards) {
+ ao2_unlock(app->forwards);
+
return -1;
}
ao2_link_flags(app->forwards, forwards, OBJ_NOLOCK);
@@ -1493,6 +1525,7 @@
/* Subscribe for messages */
messaging_app_subscribe_endpoint(app->name, endpoint, &message_received_handler, app);
}
+ ao2_unlock(app->forwards);
++forwards->interested;
ast_debug(3, "Endpoint '%s' is %d interested in %s\n",
@@ -1501,6 +1534,7 @@
app->name);
ao2_ref(forwards, -1);
+
return 0;
}
@@ -1520,12 +1554,14 @@
int app_is_subscribed_endpoint_id(struct stasis_app *app, const char *endpoint_id)
{
- RAII_VAR(struct app_forwards *, forwards, NULL, ao2_cleanup);
+ struct app_forwards *forwards;
if (ast_strlen_zero(endpoint_id)) {
endpoint_id = ENDPOINT_ALL;
}
forwards = ao2_find(app->forwards, endpoint_id, OBJ_SEARCH_KEY);
+ ao2_cleanup(forwards);
+
return forwards != NULL;
}
diff --git a/res/stasis/command.c b/res/stasis/command.c
index 05ebd7b..83a1a4c 100644
--- a/res/stasis/command.c
+++ b/res/stasis/command.c
@@ -76,21 +76,26 @@
void command_complete(struct stasis_app_command *command, int retval)
{
- SCOPED_MUTEX(lock, &command->lock);
-
+ ast_mutex_lock(&command->lock);
command->is_done = 1;
command->retval = retval;
ast_cond_signal(&command->condition);
+ ast_mutex_unlock(&command->lock);
}
int command_join(struct stasis_app_command *command)
{
- SCOPED_MUTEX(lock, &command->lock);
+ int ret;
+
+ ast_mutex_lock(&command->lock);
while (!command->is_done) {
ast_cond_wait(&command->condition, &command->lock);
}
- return command->retval;
+ ret = command->retval;
+ ast_mutex_unlock(&command->lock);
+
+ return ret;
}
void command_invoke(struct stasis_app_command *command,
diff --git a/res/stasis/control.c b/res/stasis/control.c
index 314d68a..af0b783 100644
--- a/res/stasis/control.c
+++ b/res/stasis/control.c
@@ -141,8 +141,9 @@
struct stasis_app_control *control,
struct app_control_rules *list, struct stasis_app_control_rule *obj)
{
- SCOPED_AO2LOCK(lock, control->command_queue);
+ ao2_lock(control->command_queue);
AST_LIST_INSERT_TAIL(list, obj, next);
+ ao2_unlock(control->command_queue);
}
static void app_control_unregister_rule(
@@ -150,7 +151,8 @@
struct app_control_rules *list, struct stasis_app_control_rule *obj)
{
struct stasis_app_control_rule *rule;
- SCOPED_AO2LOCK(lock, control->command_queue);
+
+ ao2_lock(control->command_queue);
AST_RWLIST_TRAVERSE_SAFE_BEGIN(list, rule, next) {
if (rule == obj) {
AST_RWLIST_REMOVE_CURRENT(next);
@@ -158,6 +160,7 @@
}
}
AST_RWLIST_TRAVERSE_SAFE_END;
+ ao2_unlock(control->command_queue);
}
/*!
@@ -501,9 +504,10 @@
struct ast_channel *chan, void *data)
{
struct stasis_app_control_mute_data *mute_data = data;
- SCOPED_CHANNELLOCK(lockvar, chan);
+ ast_channel_lock(chan);
ast_channel_suppress(control->channel, mute_data->direction, mute_data->frametype);
+ ast_channel_unlock(chan);
return 0;
}
@@ -528,9 +532,10 @@
struct ast_channel *chan, void *data)
{
struct stasis_app_control_mute_data *mute_data = data;
- SCOPED_CHANNELLOCK(lockvar, chan);
+ ast_channel_lock(chan);
ast_channel_unsuppress(control->channel, mute_data->direction, mute_data->frametype);
+ ast_channel_unlock(chan);
return 0;
}
@@ -739,7 +744,7 @@
struct ast_channel_snapshot *stasis_app_control_get_snapshot(
const struct stasis_app_control *control)
{
- RAII_VAR(struct stasis_message *, msg, NULL, ao2_cleanup);
+ struct stasis_message *msg;
struct ast_channel_snapshot *snapshot;
msg = stasis_cache_get(ast_channel_cache(), ast_channel_snapshot_type(),
@@ -752,6 +757,8 @@
ast_assert(snapshot != NULL);
ao2_ref(snapshot, +1);
+ ao2_ref(msg, -1);
+
return snapshot;
}
@@ -760,7 +767,8 @@
command_data_destructor_fn data_destructor,
app_command_can_exec_cb can_exec_fn)
{
- RAII_VAR(struct stasis_app_command *, command, NULL, ao2_cleanup);
+ int ret;
+ struct stasis_app_command *command;
if (control == NULL || control->is_done) {
/* If exec_command_on_condition fails, it calls the data_destructor.
@@ -780,7 +788,10 @@
return -1;
}
- return command_join(command);
+ ret = command_join(command);
+ ao2_ref(command, -1);
+
+ return ret;
}
int stasis_app_send_command(struct stasis_app_control *control,
@@ -793,7 +804,7 @@
stasis_app_command_cb command_fn, void *data,
command_data_destructor_fn data_destructor)
{
- RAII_VAR(struct stasis_app_command *, command, NULL, ao2_cleanup);
+ struct stasis_app_command *command;
if (control == NULL || control->is_done) {
/* If exec_command fails, it calls the data_destructor. In order to
@@ -811,18 +822,24 @@
if (!command) {
return -1;
}
+ ao2_ref(command, -1);
return 0;
}
struct ast_bridge *stasis_app_get_bridge(struct stasis_app_control *control)
{
+ struct ast_bridge *ret;
+
if (!control) {
return NULL;
- } else {
- SCOPED_AO2LOCK(lock, control);
- return control->bridge;
}
+
+ ao2_lock(control);
+ ret = control->bridge;
+ ao2_unlock(control);
+
+ return ret;
}
/*!
@@ -963,16 +980,16 @@
static int bridge_channel_depart(struct stasis_app_control *control,
struct ast_channel *chan, void *data)
{
- struct ast_bridge_channel *bridge_channel = data;
+ struct ast_bridge_channel *bridge_channel;
- {
- SCOPED_CHANNELLOCK(lock, chan);
+ ao2_lock(chan);
+ bridge_channel = ast_channel_internal_bridge_channel(chan);
+ ao2_unlock(chan);
- if (bridge_channel != ast_channel_internal_bridge_channel(chan)) {
- ast_debug(3, "%s: Channel is no longer in departable state\n",
- ast_channel_uniqueid(chan));
- return -1;
- }
+ if (bridge_channel != data) {
+ ast_debug(3, "%s: Channel is no longer in departable state\n",
+ ast_channel_uniqueid(chan));
+ return -1;
}
ast_debug(3, "%s: Channel departing bridge\n",
@@ -987,9 +1004,9 @@
enum ast_bridge_after_cb_reason reason)
{
struct stasis_app_control *control = data;
- SCOPED_AO2LOCK(lock, control);
struct ast_bridge_channel *bridge_channel;
+ ao2_lock(control);
ast_debug(3, "%s, %s: %s\n",
ast_channel_uniqueid(chan), control->bridge ? control->bridge->uniqueid : "unknown",
ast_bridge_after_cb_reason_string(reason));
@@ -1035,6 +1052,7 @@
ast_softhangup_nolock(chan, hangup_flag);
ast_channel_unlock(chan);
}
+ ao2_unlock(control);
}
static void bridge_after_cb(struct ast_channel *chan, void *data)
diff --git a/res/stasis/stasis_bridge.c b/res/stasis/stasis_bridge.c
index 7ce675c..d78e59c 100644
--- a/res/stasis/stasis_bridge.c
+++ b/res/stasis/stasis_bridge.c
@@ -250,7 +250,7 @@
{
if (src->v_table == &bridge_stasis_v_table &&
dst->v_table != &bridge_stasis_v_table) {
- RAII_VAR(struct stasis_app_control *, control, NULL, ao2_cleanup);
+ struct stasis_app_control *control;
struct ast_channel *chan;
chan = bridge_channel->chan;
@@ -263,6 +263,7 @@
stasis_app_channel_set_stasis_end_published(chan);
app_send_end_msg(control_app(control), chan);
+ ao2_ref(control, -1);
}
return -1;
--
To view, visit https://gerrit.asterisk.org/7832
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4ce67120583a446babf9adeec678b71d37fcd9e5
Gerrit-Change-Number: 7832
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180105/4d9459c5/attachment-0001.html>
More information about the asterisk-code-review
mailing list