[Asterisk-code-review] json: Audit ast json * usage for thread safety. (asterisk[13.7])
Joshua Colp
asteriskteam at digium.com
Fri Dec 18 11:09:27 CST 2015
Joshua Colp has uploaded a new change for review.
https://gerrit.asterisk.org/1842
Change subject: json: Audit ast_json_* usage for thread safety.
......................................................................
json: Audit ast_json_* usage for thread safety.
The JSON library Asterisk uses, jansson, is not thread
safe for us in a few ways. To help with this wrappers for JSON
object reference count increasing and decreasing were added
which use a global lock to ensure they don't clobber over
each other. This does not extend to reference count manipulation
within the jansson library itself. This means you can't safely
use the object borrowing specifier (O) in ast_json_pack and
you can't share JSON instances between objects.
This change removes uses of the O specifier and replaces them
with the o specifier and an explicit ast_json_ref. Some cases
of instance sharing have also been removed.
ASTERISK-25601 #close
Change-Id: I06550d8b0cc1bfeb56cab580a4e608ae4f1ec7d1
---
M main/aoc.c
M main/loader.c
M main/rtp_engine.c
M main/stasis.c
M main/stasis_channels.c
M res/res_fax.c
M res/res_stasis.c
M res/res_stasis_playback.c
M res/res_stasis_recording.c
M res/stasis/app.c
10 files changed, 49 insertions(+), 39 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/42/1842/1
diff --git a/main/aoc.c b/main/aoc.c
index 5bce066..29c5e87 100644
--- a/main/aoc.c
+++ b/main/aoc.c
@@ -1667,11 +1667,11 @@
}
return ast_json_pack(
- "{s:s, s:s, s:s, s:O}",
+ "{s:s, s:s, s:s, s:o}",
"Type", aoc_charge_type_str(decoded->charge_type),
"BillingID", aoc_billingid_str(decoded->billing_id),
"TotalType", aoc_type_of_totaling_str(decoded->total_type),
- obj_type, obj);
+ obj_type, ast_json_ref(obj));
}
static struct ast_json *association_to_json(const struct ast_aoc_decoded *decoded)
@@ -1738,10 +1738,10 @@
"Scale", decoded->aoc_s_entries[i].rate.duration.granularity_time_scale);
}
- type = ast_json_pack("{s:O, s:s, s:O, s:O}", "Currency", currency, "ChargingType",
+ type = ast_json_pack("{s:o, s:s, s:o, s:o}", "Currency", ast_json_ref(currency), "ChargingType",
decoded->aoc_s_entries[i].rate.duration.charging_type ?
- "StepFunction" : "ContinuousCharging", "Time", time,
- "Granularity", granularity ? granularity : ast_json_null());
+ "StepFunction" : "ContinuousCharging", "Time", ast_json_ref(time),
+ "Granularity", granularity ? ast_json_ref(granularity) : ast_json_ref(ast_json_null()));
break;
}
@@ -1751,7 +1751,7 @@
decoded->aoc_s_entries[i].rate.flat.amount,
decoded->aoc_s_entries[i].rate.flat.multiplier);
- type = ast_json_pack("{s:O}", "Currency", currency);
+ type = ast_json_pack("{s:o}", "Currency", ast_json_ref(currency));
break;
case AST_AOC_RATE_TYPE_VOLUME:
currency = currency_to_json(
@@ -1760,9 +1760,9 @@
decoded->aoc_s_entries[i].rate.volume.multiplier);
type = ast_json_pack(
- "{s:s, s:O}", "Unit", aoc_volume_unit_str(
+ "{s:s, s:o}", "Unit", aoc_volume_unit_str(
decoded->aoc_s_entries[i].rate.volume.volume_unit),
- "Currency", currency);
+ "Currency", ast_json_ref(currency));
break;
case AST_AOC_RATE_TYPE_SPECIAL_CODE:
type = ast_json_pack("{s:i}", "SpecialCode",
@@ -1772,8 +1772,8 @@
break;
}
- rate = ast_json_pack("{s:s, s:O}", "Chargeable", charge_item,
- aoc_rate_type_str(decoded->aoc_s_entries[i].rate_type), type);
+ rate = ast_json_pack("{s:s, s:o}", "Chargeable", charge_item,
+ aoc_rate_type_str(decoded->aoc_s_entries[i].rate_type), ast_json_ref(type));
if (ast_json_array_append(rates, rate)) {
break;
}
diff --git a/main/loader.c b/main/loader.c
index 940b53e..2b3bd1f 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -852,10 +852,10 @@
event_object = ast_json_pack("{s: s, s: s}",
"Module", S_OR(name, "All"),
"Status", res_buffer);
- json_object = ast_json_pack("{s: s, s: i, s: O}",
+ json_object = ast_json_pack("{s: s, s: i, s: o}",
"type", "Reload",
"class_type", EVENT_FLAG_SYSTEM,
- "event", event_object);
+ "event", ast_json_ref(event_object));
if (!json_object) {
return;
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 7c5ef31..6ff8f52 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -2008,12 +2008,12 @@
}
}
- json_rtcp_report = ast_json_pack("{s: i, s: i, s: i, s: O, s: O}",
+ json_rtcp_report = ast_json_pack("{s: i, s: i, s: i, s: o, s: o}",
"ssrc", payload->report->ssrc,
"type", payload->report->type,
"report_count", payload->report->reception_report_count,
- "sender_information", json_rtcp_sender_info ? json_rtcp_sender_info : ast_json_null(),
- "report_blocks", json_rtcp_report_blocks);
+ "sender_information", json_rtcp_sender_info ? ast_json_ref(json_rtcp_sender_info) : ast_json_ref(ast_json_null()),
+ "report_blocks", ast_json_ref(json_rtcp_report_blocks));
if (!json_rtcp_report) {
return NULL;
}
@@ -2025,10 +2025,10 @@
}
}
- return ast_json_pack("{s: O, s: O, s: O}",
- "channel", payload->snapshot ? json_channel : ast_json_null(),
- "rtcp_report", json_rtcp_report,
- "blob", payload->blob);
+ return ast_json_pack("{s: o, s: o, s: o}",
+ "channel", payload->snapshot ? ast_json_ref(json_channel) : ast_json_ref(ast_json_null()),
+ "rtcp_report", ast_json_ref(json_rtcp_report),
+ "blob", ast_json_deep_copy(payload->blob));
}
static void rtp_rtcp_report_dtor(void *obj)
diff --git a/main/stasis.c b/main/stasis.c
index 6adbfc3..962efc8 100644
--- a/main/stasis.c
+++ b/main/stasis.c
@@ -1274,8 +1274,8 @@
ast_json_object_set(out, "type", ast_json_string_create("ChannelUserevent"));
ast_json_object_set(out, "timestamp", ast_json_timeval(*tv, NULL));
- ast_json_object_set(out, "eventname", ast_json_ref(ast_json_object_get(blob, "eventname")));
- ast_json_object_set(out, "userevent", ast_json_ref(blob)); /* eventname gets duplicated, that's ok */
+ ast_json_object_set(out, "eventname", ast_json_string_create(ast_json_string_get((ast_json_object_get(blob, "eventname")))));
+ ast_json_object_set(out, "userevent", ast_json_deep_copy(blob));
for (type = 0; type < STASIS_UMOS_MAX; ++type) {
for (i = 0; i < AST_VECTOR_SIZE(&multi->snapshots[type]); ++i) {
diff --git a/main/stasis_channels.c b/main/stasis_channels.c
index b8efd40..d46a8dd 100644
--- a/main/stasis_channels.c
+++ b/main/stasis_channels.c
@@ -1016,6 +1016,10 @@
struct ast_channel_snapshot *snapshot = channel_blob->snapshot;
const char *direction =
ast_json_string_get(ast_json_object_get(blob, "direction"));
+ const char *digit =
+ ast_json_string_get(ast_json_object_get(blob, "digit"));
+ long duration_ms =
+ ast_json_integer_get(ast_json_object_get(blob, "duration_ms"));
const struct timeval *tv = stasis_message_timestamp(message);
struct ast_json *json_channel;
@@ -1029,11 +1033,11 @@
return NULL;
}
- return ast_json_pack("{s: s, s: o, s: O, s: O, s: o}",
+ return ast_json_pack("{s: s, s: o, s: s, s: i, s: o}",
"type", "ChannelDtmfReceived",
"timestamp", ast_json_timeval(*tv, NULL),
- "digit", ast_json_object_get(blob, "digit"),
- "duration_ms", ast_json_object_get(blob, "duration_ms"),
+ "digit", digit,
+ "duration_ms", duration_ms,
"channel", json_channel);
}
@@ -1057,6 +1061,12 @@
{
struct ast_multi_channel_blob *payload = stasis_message_data(message);
struct ast_json *blob = ast_multi_channel_blob_get_json(payload);
+ const char *dialstatus =
+ ast_json_string_get(ast_json_object_get(blob, "dialstatus"));
+ const char *forward =
+ ast_json_string_get(ast_json_object_get(blob, "forward"));
+ const char *dialstring =
+ ast_json_string_get(ast_json_object_get(blob, "dialstring"));
struct ast_json *caller_json = ast_channel_snapshot_to_json(ast_multi_channel_blob_get_channel(payload, "caller"), sanitize);
struct ast_json *peer_json = ast_channel_snapshot_to_json(ast_multi_channel_blob_get_channel(payload, "peer"), sanitize);
struct ast_json *forwarded_json = ast_channel_snapshot_to_json(ast_multi_channel_blob_get_channel(payload, "forwarded"), sanitize);
@@ -1064,12 +1074,12 @@
const struct timeval *tv = stasis_message_timestamp(message);
int res = 0;
- json = ast_json_pack("{s: s, s: o, s: O, s: O, s: O}",
+ json = ast_json_pack("{s: s, s: o, s: s, s: s, s: s}",
"type", "Dial",
"timestamp", ast_json_timeval(*tv, NULL),
- "dialstatus", ast_json_object_get(blob, "dialstatus"),
- "forward", ast_json_object_get(blob, "forward"),
- "dialstring", ast_json_object_get(blob, "dialstring"));
+ "dialstatus", dialstatus,
+ "forward", forward,
+ "dialstring", dialstring);
if (!json) {
ast_json_unref(caller_json);
ast_json_unref(peer_json);
diff --git a/res/res_fax.c b/res/res_fax.c
index e947c91..ef0e276 100644
--- a/res/res_fax.c
+++ b/res/res_fax.c
@@ -2028,14 +2028,14 @@
fax_bitrate = ast_strdupa(fax_bitrate);
}
- json_object = ast_json_pack("{s: s, s: s, s: s, s: s, s: s, s: s, s: O}",
+ json_object = ast_json_pack("{s: s, s: s, s: s, s: s, s: s, s: s, s: o}",
"type", "receive",
"remote_station_id", S_OR(remote_station_id, ""),
"local_station_id", S_OR(local_station_id, ""),
"fax_pages", S_OR(fax_pages, ""),
"fax_resolution", S_OR(fax_resolution, ""),
"fax_bitrate", S_OR(fax_bitrate, ""),
- "filenames", json_array);
+ "filenames", ast_json_ref(json_array));
if (!json_object) {
return -1;
}
diff --git a/res/res_stasis.c b/res/res_stasis.c
index 1cab8c3..94b037e 100644
--- a/res/res_stasis.c
+++ b/res/res_stasis.c
@@ -150,10 +150,10 @@
return NULL;
}
- msg = ast_json_pack("{s: s, s: O, s: O, s: o}",
+ msg = ast_json_pack("{s: s, s: o, s: o, s: o}",
"type", "StasisStart",
- "timestamp", ast_json_object_get(payload->blob, "timestamp"),
- "args", ast_json_object_get(payload->blob, "args"),
+ "timestamp", ast_json_copy(ast_json_object_get(payload->blob, "timestamp")),
+ "args", ast_json_deep_copy(ast_json_object_get(payload->blob, "args")),
"channel", ast_channel_snapshot_to_json(payload->channel, NULL));
if (!msg) {
ast_log(LOG_ERROR, "Failed to pack JSON for StasisStart message\n");
diff --git a/res/res_stasis_playback.c b/res/res_stasis_playback.c
index dc6e8f2..779dd77 100644
--- a/res/res_stasis_playback.c
+++ b/res/res_stasis_playback.c
@@ -105,9 +105,9 @@
return NULL;
}
- return ast_json_pack("{s: s, s: O}",
+ return ast_json_pack("{s: s, s: o}",
"type", type,
- "playback", blob);
+ "playback", ast_json_deep_copy(blob));
}
STASIS_MESSAGE_TYPE_DEFN(stasis_app_playback_snapshot_type,
diff --git a/res/res_stasis_recording.c b/res/res_stasis_recording.c
index 50e0697..392d92c 100644
--- a/res/res_stasis_recording.c
+++ b/res/res_stasis_recording.c
@@ -91,9 +91,9 @@
return NULL;
}
- return ast_json_pack("{s: s, s: O}",
+ return ast_json_pack("{s: s, s: o}",
"type", type,
- "recording", blob);
+ "recording", ast_json_deep_copy(blob));
}
STASIS_MESSAGE_TYPE_DEFN(stasis_app_recording_snapshot_type,
diff --git a/res/stasis/app.c b/res/stasis/app.c
index 3539182..4e18aa5 100644
--- a/res/stasis/app.c
+++ b/res/stasis/app.c
@@ -610,11 +610,11 @@
return -1;
}
- app_send(app, ast_json_pack("{s: s, s: o, s: o, s: O}",
+ app_send(app, ast_json_pack("{s: s, s: o, s: o, s: o}",
"type", "TextMessageReceived",
"timestamp", ast_json_timeval(ast_tvnow(), NULL),
"endpoint", json_endpoint,
- "message", json_msg));
+ "message", ast_json_ref(json_msg)));
return 0;
}
--
To view, visit https://gerrit.asterisk.org/1842
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I06550d8b0cc1bfeb56cab580a4e608ae4f1ec7d1
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13.7
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
More information about the asterisk-code-review
mailing list