conversion from string to JSON is not multithread-safe

[bhaible]

When two different threads use json_loads to convert a string to a JSON object, they can disturb each other: one of the threads may run into an assertion failure and crash the program.

How to reproduce:
On a system with glibc, compile and run the following program foo.c:

#include <locale.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <jansson.h>

static void *
thread1_func (void *arg)
{
  locale_t thread1_locale = newlocale (LC_ALL_MASK, "en_US.UTF-8", NULL);
  uselocale (thread1_locale);

  char input[] = "{\"a\":1.5}";

  for (;;)
    {
      json_error_t e;
      json_t *j = json_loads (input, 0, &e);
      if (!json_is_object (j))
        abort ();
      json_t *k = json_object_get (j, "a");
      if (!json_is_real (k))
        abort ();
      if (json_real_value (k) != 1.5)
        {
          fprintf (stderr, "thread1 disturbed by thread2, read %g\n", json_real_value (k)); fflush (stderr);
          abort ();
        }
      json_decref (j);
    }

  /*NOTREACHED*/
}

static void *
thread2_func (void *arg)
{
  locale_t thread2_locale = newlocale (LC_ALL_MASK, "fr_FR.UTF-8", NULL);
  uselocale (thread2_locale);

  char input[] = "{\"b\":2.5}";

  for (;;)
    {
      json_error_t e;
      json_t *j = json_loads (input, 0, &e);
      if (!json_is_object (j))
        abort ();
      json_t *k = json_object_get (j, "b");
      if (!json_is_real (k))
        abort ();
      if (json_real_value (k) != 2.5)
        {
          fprintf (stderr, "thread2 disturbed by thread1, read %g\n", json_real_value (k)); fflush (stderr);
          abort ();
        }
      json_decref (j);
    }

  /*NOTREACHED*/
}

int
main (int argc, char *argv[])
{
  /* Create the threads.  */
  pthread_t thread1, thread2;
  pthread_create (&thread1, NULL, thread1_func, NULL);
  pthread_create (&thread2, NULL, thread2_func, NULL);

  /* Let them run for 1 second.  */
  {
    struct timespec duration;
    duration.tv_sec = (argc > 1 ? atoi (argv[1]) : 1);
    duration.tv_nsec = 0;

    nanosleep (&duration, NULL);
  }

  return 0;
}

Like this:

$ gcc -Wall foo.c /usr/lib/x86_64-linux-gnu/libjansson.a -lpthread
$ ./a.out
a.out: strconv.c:68: jsonp_strtod: Assertion `end == strbuffer->value + strbuffer->length' failed.
Aborted (core dumped)

Here is the gdb stack trace:

(gdb) where
#0  0x00007ffff7e079fc in pthread_kill () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007ffff7db3476 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007ffff7d997f3 in abort () from /lib/x86_64-linux-gnu/libc.so.6
#3  0x00007ffff7d9971b in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#4  0x00007ffff7daae96 in __assert_fail () from /lib/x86_64-linux-gnu/libc.so.6
#5  0x0000555555557ae5 in jsonp_strtod ()
#6  0x0000555555556670 in lex_scan.isra ()
#7  0x0000555555556b14 in parse_value ()
#8  0x0000555555556e65 in parse_json ()
#9  0x0000555555556feb in json_loads ()
#10 0x000055555555564c in thread1_func ()

Note: When one of the pthread_create lines is commented out, such that only one thread is created, the program runs fine. Only when both pthread_create lines are enabled, does the program crash. This proves that there is an interaction between the threads.

Note: The test program fulfils the rules documented in https://jansson.readthedocs.io/en/latest/threadsafety.html :

• The json_t objects are private to each of the threads.
• It does not invoke setlocale.

[bhaible]

The cause is that the conversion from string to double, in strconv.c: jsonp_strtod, uses the localeconv() function, which is not multithread-safe (see POSIX https://pubs.opengroup.org/onlinepubs/9699919799/functions/localeconv.html ). In particular, in glibc, localeconv() returns a pointer to a static variable.

So, in thread1 with its English locale, localeconv()->decimal_point is ".", whereas in thread2 with its French locale, localeconv()->decimal_point is ",".

In thread1, when jsonp_strtod is called on "1.5", it first calls to_locale(strbuffer). Here, it can happen that the return value of localeconv() is disturbed by thread2, such that localeconv()->decimal_point returns ",". In this case, to_locale produces the string "1,5". When strtod is called on this string, it returns the number 1, and the end pointer points to the comma (rather than to the end of the string).

[bhaible]

The fix is to use sprintf (buffer, "%#.0f", 1.0) instead of localeconv(). sprintf is multithread-safe.

[akheron]

Thanks for the detailed bug report @bhaible! Do you think #677 would be the correct fix?

[bhaible]

Do you think #677 would be the correct fix?

src/strconv.c: The code change is correct, yes. But I would keep the comment in lines 17..25, since it is still valid and relevant even after this fix.

test/bin/json_process.c, test/suites/api/util.h: These changes needlessly reduce the test coverage. Before, with the setlocale (LC_ALL, "") invocation, the test would use the "C" locale or an English locale on some machines, and a French or German locale (with a comma as decimal-point character) on other machines. If you remove the setlocale (LC_ALL, "") invocation, the test uses the "C" locale always; this provides less coverage of relevant test scenarios.

[akheron]

Thanks! Fixed in #677.