[Asterisk-code-review] json: Add conditionals to avoid locking if Jansson is thread... (asterisk[13])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/8186 )

Change subject: json: Add conditionals to avoid locking if Jansson is thread safe.
......................................................................

json: Add conditionals to avoid locking if Jansson is thread safe.

Jansson is thread safe for all read-only functions and reference
counting starting v2.11.  This allows simplification of our code and
removal of locking around reference counting and dumping.

Change-Id: Id985cb3ffa6681f9ac765642e20fcd187bd4aeee
---
M contrib/scripts/install_prereq
M main/json.c
2 files changed, 42 insertions(+), 6 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/contrib/scripts/install_prereq b/contrib/scripts/install_prereq
index 1ca640c..3df7c03 100755
--- a/contrib/scripts/install_prereq
+++ b/contrib/scripts/install_prereq
@@ -54,7 +54,7 @@
 
 KVERS=`uname -r`
 
-JANSSON_VER=2.10
+JANSSON_VER=2.11
 
 case "$1" in
 test)
diff --git a/main/json.c b/main/json.c
index 057a262..1837b5b 100644
--- a/main/json.c
+++ b/main/json.c
@@ -46,6 +46,21 @@
 #include <jansson.h>
 #include <time.h>
 
+#if defined(JANSSON_THREAD_SAFE_REFCOUNT)
+void *ast_json_malloc(size_t size)
+{
+	return ast_malloc(size);
+}
+
+void ast_json_free(void *p)
+{
+	ast_free(p);
+}
+
+/* No need to lock since jansson is thread safe. */
+#define SCOPED_JSON_LOCK(json)
+
+#else
 /*! \brief Magic number, for safety checks. */
 #define JSON_MAGIC 0x1541992
 
@@ -189,6 +204,7 @@
 
 	AST_LIST_INSERT_HEAD(free_list, mem, list);
 }
+#endif
 
 void ast_json_set_alloc_funcs(void *(*malloc_fn)(size_t), void (*free_fn)(void*))
 {
@@ -202,7 +218,7 @@
 
 struct ast_json *ast_json_ref(struct ast_json *json)
 {
-	/* Jansson refcounting is non-atomic; lock it. */
+	/* If Jansson refcounting is non-atomic; lock it. */
 	SCOPED_JSON_LOCK(json);
 	json_incref((json_t *)json);
 	return json;
@@ -210,6 +226,9 @@
 
 void ast_json_unref(struct ast_json *json)
 {
+#if defined(JANSSON_THREAD_SAFE_REFCOUNT)
+	json_decref((json_t *) json);
+#else
 	struct json_mem_list *free_list;
 	struct json_mem *mem;
 
@@ -234,6 +253,7 @@
 	while ((mem = AST_LIST_REMOVE_HEAD(free_list, list))) {
 		json_mem_free(mem);
 	}
+#endif
 }
 
 enum ast_json_type ast_json_typeof(const struct ast_json *json)
@@ -666,7 +686,11 @@
 {
 	/* Jansson's json_dump*, even though it's a read operation, isn't
 	 * thread safe for concurrent reads. Locking is necessary.
-	 * See http://www.digip.org/jansson/doc/2.4/portability.html#thread-safety. */
+	 * See http://www.digip.org/jansson/doc/2.4/portability.html#thread-safety.
+	 *
+	 * This comment does not apply when JANSSON_THREAD_SAFE_REFCOUNT is defined,
+	 * in that case SCOPED_JSON_LOCK is a no-op.
+	 */
 	SCOPED_JSON_LOCK(root);
 	return json_dumps((json_t *)root, dump_flags(format));
 }
@@ -706,7 +730,11 @@
 {
 	/* Jansson's json_dump*, even though it's a read operation, isn't
 	 * thread safe for concurrent reads. Locking is necessary.
-	 * See http://www.digip.org/jansson/doc/2.4/portability.html#thread-safety. */
+	 * See http://www.digip.org/jansson/doc/2.4/portability.html#thread-safety.
+	 *
+	 * This comment does not apply when JANSSON_THREAD_SAFE_REFCOUNT is defined,
+	 * in that case SCOPED_JSON_LOCK is a no-op.
+	 */
 	SCOPED_JSON_LOCK(root);
 	return json_dump_callback((json_t *)root, write_to_ast_str, dst, dump_flags(format));
 }
@@ -716,7 +744,11 @@
 {
 	/* Jansson's json_dump*, even though it's a read operation, isn't
 	 * thread safe for concurrent reads. Locking is necessary.
-	 * See http://www.digip.org/jansson/doc/2.4/portability.html#thread-safety. */
+	 * See http://www.digip.org/jansson/doc/2.4/portability.html#thread-safety.
+	 *
+	 * This comment does not apply when JANSSON_THREAD_SAFE_REFCOUNT is defined,
+	 * in that case SCOPED_JSON_LOCK is a no-op.
+	 */
 	SCOPED_JSON_LOCK(root);
 	if (!root || !output) {
 		return -1;
@@ -727,7 +759,11 @@
 {
 	/* Jansson's json_dump*, even though it's a read operation, isn't
 	 * thread safe for concurrent reads. Locking is necessary.
-	 * See http://www.digip.org/jansson/doc/2.4/portability.html#thread-safety. */
+	 * See http://www.digip.org/jansson/doc/2.4/portability.html#thread-safety.
+	 *
+	 * This comment does not apply when JANSSON_THREAD_SAFE_REFCOUNT is defined,
+	 * in that case SCOPED_JSON_LOCK is a no-op.
+	 */
 	SCOPED_JSON_LOCK(root);
 	if (!root || !path) {
 		return -1;

-- 
To view, visit https://gerrit.asterisk.org/8186
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: merged
Gerrit-Change-Id: Id985cb3ffa6681f9ac765642e20fcd187bd4aeee
Gerrit-Change-Number: 8186
Gerrit-PatchSet: 3
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180213/cfb90ca6/attachment-0001.html>