From 23ee243113ac47730c7ca900b35a1abbcb568743 Mon Sep 17 00:00:00 2001 From: Emiel Bruijntjes Date: Fri, 27 Nov 2015 16:49:32 +0100 Subject: [PATCH 01/11] reference increment and decrement is now atomic (when using a GCC compatible compiler), which allows passing json objects between threads --- json_object.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/json_object.c b/json_object.c index 9ac22a3..cad3137 100644 --- a/json_object.c +++ b/json_object.c @@ -160,25 +160,29 @@ static int json_escape_str(struct printbuf *pb, const char *str, int len) extern struct json_object* json_object_get(struct json_object *jso) { - if (jso) - jso->_ref_count++; + if (!jso) return jso; +#if defined __GNUC__ + __sync_add_and_fetch(&jso->_ref_count, 1); +#else + ++jso->_ref_count; +#endif return jso; } int json_object_put(struct json_object *jso) { - if(jso) - { - jso->_ref_count--; - if(!jso->_ref_count) - { - if (jso->_user_delete) - jso->_user_delete(jso, jso->_userdata); - jso->_delete(jso); - return 1; - } - } - return 0; + if(!jso) return 0; + +#if defined __GNUC__ + if (__sync_fetch_and_sub(&jso->_ref_count, 1) > 0) return 0; +#else + if (--jso->_ref_count > 0) return 0; +#endif + + if (jso->_user_delete) + jso->_user_delete(jso, jso->_userdata); + jso->_delete(jso); + return 1; } From 827f0fd8ef57b71dce7910325b6ce541bd6d0b50 Mon Sep 17 00:00:00 2001 From: Emiel Bruijntjes Date: Fri, 27 Nov 2015 16:52:17 +0100 Subject: [PATCH 02/11] update indentation --- json_object.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/json_object.c b/json_object.c index cad3137..2b9bef7 100644 --- a/json_object.c +++ b/json_object.c @@ -161,11 +161,13 @@ static int json_escape_str(struct printbuf *pb, const char *str, int len) extern struct json_object* json_object_get(struct json_object *jso) { if (!jso) return jso; + #if defined __GNUC__ __sync_add_and_fetch(&jso->_ref_count, 1); #else ++jso->_ref_count; #endif + return jso; } From 9d8536767970bb78abc2f0fc26e9ca8fdf35bd6b Mon Sep 17 00:00:00 2001 From: Emiel Bruijntjes Date: Fri, 27 Nov 2015 16:53:57 +0100 Subject: [PATCH 03/11] added tabs instead of spaces to be compatible with rest of code --- json_object.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/json_object.c b/json_object.c index 2b9bef7..eae0f8a 100644 --- a/json_object.c +++ b/json_object.c @@ -163,9 +163,9 @@ extern struct json_object* json_object_get(struct json_object *jso) if (!jso) return jso; #if defined __GNUC__ - __sync_add_and_fetch(&jso->_ref_count, 1); + __sync_add_and_fetch(&jso->_ref_count, 1); #else - ++jso->_ref_count; + ++jso->_ref_count; #endif return jso; @@ -176,15 +176,15 @@ int json_object_put(struct json_object *jso) if(!jso) return 0; #if defined __GNUC__ - if (__sync_fetch_and_sub(&jso->_ref_count, 1) > 0) return 0; + if (__sync_fetch_and_sub(&jso->_ref_count, 1) > 0) return 0; #else - if (--jso->_ref_count > 0) return 0; + if (--jso->_ref_count > 0) return 0; #endif - if (jso->_user_delete) - jso->_user_delete(jso, jso->_userdata); - jso->_delete(jso); - return 1; + if (jso->_user_delete) + jso->_user_delete(jso, jso->_userdata); + jso->_delete(jso); + return 1; } From 7e98ed93f4942a1baade1bade36a67c77b43ec85 Mon Sep 17 00:00:00 2001 From: Emiel Bruijntjes Date: Sat, 28 Nov 2015 14:19:43 +0100 Subject: [PATCH 04/11] subtract first, then retrieve value --- json_object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/json_object.c b/json_object.c index eae0f8a..b7ca93b 100644 --- a/json_object.c +++ b/json_object.c @@ -176,7 +176,7 @@ int json_object_put(struct json_object *jso) if(!jso) return 0; #if defined __GNUC__ - if (__sync_fetch_and_sub(&jso->_ref_count, 1) > 0) return 0; + if (__sync_sub_and_fetch(&jso->_ref_count, 1) > 0) return 0; #else if (--jso->_ref_count > 0) return 0; #endif From 447d67d5f310d7275e3113475099bfcb0b184824 Mon Sep 17 00:00:00 2001 From: Eric Haszlakiewicz Date: Wed, 30 Aug 2017 23:17:24 -0400 Subject: [PATCH 05/11] Issue #349: none of automake's clean targets are suite for really cleaning up everything, so add a local "really-clean" target that does so. --- Makefile.am | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index bb4fb6a..cd32b4c 100644 --- a/Makefile.am +++ b/Makefile.am @@ -56,13 +56,43 @@ libjson_c_la_SOURCES = \ strerror_override.c \ strerror_override_private.h + +DISTCLEANFILES= +DISTCLEANFILES+= \ + config.h \ + json-c-uninstalled.pc \ + json-c.pc \ + json_config.h + distclean-local: -rm -rf $(testsubdir) - -rm -rf config.h.in~ Makefile.in aclocal.m4 autom4te.cache/ config.guess config.sub depcomp install-sh ltmain.sh missing - -rm -f INSTALL test-driver tests/Makefile.in compile -maintainer-clean-local: - -rm -rf configure +JSON_CLEANFILES= +JSON_CLEANFILES+= \ + Makefile.in \ + aclocal.m4 \ + autom4te.cache/ \ + compile \ + config.guess \ + config.h.in \ + config.sub \ + configure \ + depcomp \ + install-sh \ + ltmain.sh \ + missing \ + test-driver \ + tests/Makefile.in +JSON_CLEANFILES+= \ + libtool \ + stamp-h1 \ + stamp-h2 + +# There's no built-in way to remove these after all the other +# maintainer-clean steps happen, so do it explicitly here. +really-clean: + $(MAKE) maintainer-clean + rm -rf ${JSON_CLEANFILES} uninstall-local: rm -rf "$(DESTDIR)@includedir@/json-c" From 95dff31951c9388408d29e7ddf9a9edc4ec96dca Mon Sep 17 00:00:00 2001 From: Eric Haszlakiewicz Date: Wed, 30 Aug 2017 23:35:56 -0400 Subject: [PATCH 06/11] Issue #351: don't redefine SIZE_T_MAX if it's already defined. --- arraylist.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arraylist.c b/arraylist.c index 8439cc2..ddeb8d4 100644 --- a/arraylist.c +++ b/arraylist.c @@ -22,6 +22,7 @@ # include #endif /* HAVE_STRINGS_H */ +#ifndef SIZE_T_MAX #if SIZEOF_SIZE_T == SIZEOF_INT #define SIZE_T_MAX UINT_MAX #elif SIZEOF_SIZE_T == SIZEOF_LONG @@ -31,6 +32,7 @@ #else #error Unable to determine size of size_t #endif +#endif #include "arraylist.h" From 5b11e9adffd57682fdd9d46598c92bb52016047a Mon Sep 17 00:00:00 2001 From: Eric Haszlakiewicz Date: Sat, 2 Sep 2017 14:48:17 -0400 Subject: [PATCH 07/11] Explicitly check for GCC's atomic functions instead of depending on the __GNUC__ define. Add a comment mentioning the limitation even though the _ref_count value is hanled atomically. --- configure.ac | 24 ++++++++++++++++++++++++ json_object.c | 10 ++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index bbaacf5..4f507ff 100644 --- a/configure.ac +++ b/configure.ac @@ -66,6 +66,30 @@ AC_CHECK_DECLS([isnan], [], [], [[#include ]]) AC_CHECK_DECLS([isinf], [], [], [[#include ]]) AC_CHECK_DECLS([_isnan], [], [], [[#include ]]) AC_CHECK_DECLS([_finite], [], [], [[#include ]]) +AC_MSG_CHECKING(for GCC atomic builtins) +AC_LINK_IFELSE( +[ + AC_LANG_SOURCE([[ + int main() { + volatile unsigned int val = 1; + /* Note: __sync_val_compare_and_swap isn't checked here + * because it's protected by __GCC_HAVE_SYNC_COMPARE_AND_SWAP_, + * which is automatically defined by gcc. + */ + __sync_add_and_fetch(&val, 1); + __sync_sub_and_fetch(&val, 1); + return 0; + } + ]]) +], +[ + AC_MSG_RESULT([yes]) + AC_DEFINE([HAVE_ATOMIC_BUILTINS],[1],[Has atomic builtins]) +], +[ + AC_MSG_RESULT([no]) + AC_MSG_WARN([json-c will be built without atomic refcounts because atomic builtins are missing]) +]) case "${host_os}" in linux*) diff --git a/json_object.c b/json_object.c index 028d495..c09d61d 100644 --- a/json_object.c +++ b/json_object.c @@ -165,7 +165,7 @@ extern struct json_object* json_object_get(struct json_object *jso) { if (!jso) return jso; -#if defined __GNUC__ +#ifdef HAVE_ATOMIC_BUILTINS __sync_add_and_fetch(&jso->_ref_count, 1); #else ++jso->_ref_count; @@ -178,7 +178,13 @@ int json_object_put(struct json_object *jso) { if(!jso) return 0; -#if defined __GNUC__ +#ifdef HAVE_ATOMIC_BUILTINS + /* Note: this only allow the refcount to remain correct + * when multiple threads are adjusting it. It is still an error + * for a thread to decrement the refcount if it doesn't "own" it, + * as that can result in the thread that loses the race to 0 + * operating on an already-freed object. + */ if (__sync_sub_and_fetch(&jso->_ref_count, 1) > 0) return 0; #else if (--jso->_ref_count > 0) return 0; From 8777c9477a86beeeffe87afe7d3991b3a9298862 Mon Sep 17 00:00:00 2001 From: Eric Haszlakiewicz Date: Sun, 3 Sep 2017 22:35:58 -0400 Subject: [PATCH 08/11] Use AC_CONFIG_MACRO_DIRS to specify path to the ax macros instead of passing -I to autoreconf in autogen.sh. --- autogen.sh | 2 +- configure.ac | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/autogen.sh b/autogen.sh index ae2a3e1..69e765a 100755 --- a/autogen.sh +++ b/autogen.sh @@ -1,5 +1,5 @@ #!/bin/sh -autoreconf -Iautoconf-archive/m4 -v --install || exit 1 +autoreconf -v --install || exit 1 # If there are any options, assume the user wants to run configure. # To run configure w/o any options, use ./autogen.sh --configure diff --git a/configure.ac b/configure.ac index 4f507ff..e909cf2 100644 --- a/configure.ac +++ b/configure.ac @@ -5,6 +5,8 @@ AC_INIT([json-c], 0.12.99, [json-c@googlegroups.com]) AM_INIT_AUTOMAKE +AC_CONFIG_MACRO_DIRS([autoconf-archive/m4]) + AC_PROG_MAKE_SET AC_CANONICAL_HOST From 2d1da5ab13463dc52c9e8d262cb51b80c7376ebb Mon Sep 17 00:00:00 2001 From: Eric Haszlakiewicz Date: Sun, 3 Sep 2017 23:37:12 -0400 Subject: [PATCH 09/11] Add a --enable-threading configure option, and only use the (slower) __sync_add_and_fetch()/__sync_sub_and_fetch() function when it is specified. --- README.md | 21 ++++++++++++++++++++- configure.ac | 16 +++++++++++++++- json_object.c | 12 ++++++------ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 0e87cd5..bddcff7 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ $ sh autogen.sh followed by ```sh -$ ./configure +$ ./configure # --enable-threading $ make $ make install ``` @@ -56,6 +56,25 @@ To build and run the test programs: $ make check ``` +Building with partial threading support +---------------------------------------- + +Although json-c does not support fully multi-threaded access to +object trees, it has some code to help make use in threaded programs +a bit safer. Currently, this is limited to using atomic operations for +json_object_get() and json_object_put(). + +Since this may have a performance impact, of at least 3x slower +according to https://stackoverflow.com/a/11609063, it is disabled by +default. You may turn it on by adjusting your configure command with: + --enable-threading + +Separately, the default hash function used for object field keys, +lh_char_hash, uses a compare-and-swap operation to ensure the randomly +seed is only generated once. Because this is a one-time operation, it +is always compiled in when the compare-and-swap operation is available. + + Linking to `libjson-c` ---------------------- diff --git a/configure.ac b/configure.ac index e909cf2..0ebf823 100644 --- a/configure.ac +++ b/configure.ac @@ -11,12 +11,26 @@ AC_PROG_MAKE_SET AC_CANONICAL_HOST +AC_ARG_ENABLE(threading, + AS_HELP_STRING([--enable-threading], + [Enable code to support partly multi-threaded use]), +[if test x$enableval = xyes; then + enable_threading=yes + AC_DEFINE(ENABLE_THREADING, 1, [Enable partial threading support]) +fi]) + +if test "x$enable_threading" = "xyes"; then + AC_MSG_RESULT([Partial multi-threaded support enabled.]) +else + AC_MSG_RESULT([Multi-threaded support disabled. Use --enable-threading to enable.]) +fi + AC_ARG_ENABLE(rdrand, AS_HELP_STRING([--enable-rdrand], [Enable RDRAND Hardware RNG Hash Seed generation on supported x86/x64 platforms.]), [if test x$enableval = xyes; then enable_rdrand=yes - AC_DEFINE(ENABLE_RDRAND, 1, [Enable RDRANR Hardware RNG Hash Seed]) + AC_DEFINE(ENABLE_RDRAND, 1, [Enable RDRAND Hardware RNG Hash Seed]) fi]) if test "x$enable_rdrand" = "xyes"; then diff --git a/json_object.c b/json_object.c index c09d61d..fcda69f 100644 --- a/json_object.c +++ b/json_object.c @@ -165,7 +165,7 @@ extern struct json_object* json_object_get(struct json_object *jso) { if (!jso) return jso; -#ifdef HAVE_ATOMIC_BUILTINS +#if defined(HAVE_ATOMIC_BUILTINS) && defined(ENABLE_THREADING) __sync_add_and_fetch(&jso->_ref_count, 1); #else ++jso->_ref_count; @@ -178,7 +178,7 @@ int json_object_put(struct json_object *jso) { if(!jso) return 0; -#ifdef HAVE_ATOMIC_BUILTINS +#if defined(HAVE_ATOMIC_BUILTINS) && defined(ENABLE_THREADING) /* Note: this only allow the refcount to remain correct * when multiple threads are adjusting it. It is still an error * for a thread to decrement the refcount if it doesn't "own" it, @@ -703,7 +703,7 @@ int json_object_set_int64(struct json_object *jso,int64_t new_value){ /* json_object_double */ -#ifdef HAVE___THREAD +#if defined(HAVE___THREAD) && defined(ENABLE_THREADING) // i.e. __thread or __declspec(thread) static SPEC___THREAD char *tls_serialization_float_format = NULL; #endif @@ -713,7 +713,7 @@ int json_c_set_serialization_double_format(const char *double_format, int global { if (global_or_thread == JSON_C_OPTION_GLOBAL) { -#ifdef HAVE___THREAD +#if defined(HAVE___THREAD) && defined(ENABLE_THREADING) if (tls_serialization_float_format) { free(tls_serialization_float_format); @@ -726,7 +726,7 @@ int json_c_set_serialization_double_format(const char *double_format, int global } else if (global_or_thread == JSON_C_OPTION_THREAD) { -#ifdef HAVE___THREAD +#if defined(HAVE___THREAD) && defined(ENABLE_THREADING) if (tls_serialization_float_format) { free(tls_serialization_float_format); @@ -775,7 +775,7 @@ static int json_object_double_to_json_string_format(struct json_object* jso, { const char *std_format = "%.17g"; -#ifdef HAVE___THREAD +#if defined(HAVE___THREAD) && defined(ENABLE_THREADING) if (tls_serialization_float_format) std_format = tls_serialization_float_format; else From b2afca45602cc856f2367e77ca842606aface4ec Mon Sep 17 00:00:00 2001 From: Eric Haszlakiewicz Date: Tue, 5 Sep 2017 01:53:13 -0400 Subject: [PATCH 10/11] Issue #173: since some sscanf implementations return 0 for non-zero inputs, directly check for "0" in the input. --- json_util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/json_util.c b/json_util.c index 2121150..4624668 100644 --- a/json_util.c +++ b/json_util.c @@ -265,7 +265,9 @@ int json_parse_int64(const char *buf, int64_t *retval) // Skip leading zeros, but keep at least one digit while (buf_sig_digits[0] == '0' && buf_sig_digits[1] != '\0') buf_sig_digits++; - if (num64 == 0) // assume all sscanf impl's will parse -0 to 0 + // Can't check num64==0 because some sscanf impl's parse + // non-zero values to 0. (e.g. Illumos with UINT64_MAX) + if (buf_sig_digits[0] == '0' && buf_sig_digits[1] == '\0') orig_has_neg = 0; // "-0" is the same as just plain "0" snprintf(buf_cmp_start, sizeof(buf_cmp), "%" PRId64, num64); From 548d000891ed2e24d6b0e267079a6e9a853726b8 Mon Sep 17 00:00:00 2001 From: Eric Haszlakiewicz Date: Tue, 5 Sep 2017 01:56:42 -0400 Subject: [PATCH 11/11] Undo a bit of 2d1da5ab: handle per-thread formats for double serialization, even if --enable-threading wasn't specified. --- json_object.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/json_object.c b/json_object.c index fcda69f..ec4e2b6 100644 --- a/json_object.c +++ b/json_object.c @@ -703,7 +703,7 @@ int json_object_set_int64(struct json_object *jso,int64_t new_value){ /* json_object_double */ -#if defined(HAVE___THREAD) && defined(ENABLE_THREADING) +#if defined(HAVE___THREAD) // i.e. __thread or __declspec(thread) static SPEC___THREAD char *tls_serialization_float_format = NULL; #endif @@ -713,7 +713,7 @@ int json_c_set_serialization_double_format(const char *double_format, int global { if (global_or_thread == JSON_C_OPTION_GLOBAL) { -#if defined(HAVE___THREAD) && defined(ENABLE_THREADING) +#if defined(HAVE___THREAD) if (tls_serialization_float_format) { free(tls_serialization_float_format); @@ -726,7 +726,7 @@ int json_c_set_serialization_double_format(const char *double_format, int global } else if (global_or_thread == JSON_C_OPTION_THREAD) { -#if defined(HAVE___THREAD) && defined(ENABLE_THREADING) +#if defined(HAVE___THREAD) if (tls_serialization_float_format) { free(tls_serialization_float_format); @@ -775,7 +775,7 @@ static int json_object_double_to_json_string_format(struct json_object* jso, { const char *std_format = "%.17g"; -#if defined(HAVE___THREAD) && defined(ENABLE_THREADING) +#if defined(HAVE___THREAD) if (tls_serialization_float_format) std_format = tls_serialization_float_format; else