akheron / jansson

C library for encoding, decoding and manipulating JSON data
http://www.digip.org/jansson/
Other
3.02k stars 807 forks source link

conversion from JSON to string is not multithread-safe #674

Closed bhaible closed 4 months ago

bhaible commented 4 months ago

When two different threads use json_dumps to convert a JSON object to a string, they can disturb each other: one of the threads may produce wrong results.

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);

  json_t *j = json_object ();
  json_object_set (j, "a", json_real (1.5));

  for (;;)
    {
      char *s = json_dumps (j, 0);
      if (strcmp (s, "{\"a\": 1.5}") != 0)
        {
          fprintf (stderr, "thread1 disturbed by thread2, produced %s\n", s); fflush (stderr);
          abort ();
        }
      free (s);
    }

  /*NOTREACHED*/
}

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

  json_t *j = json_object ();
  json_object_set (j, "b", json_real (2.5));

  for (;;)
    {
      char *s = json_dumps (j, 0);
      fprintf (stderr, "thread2: %s\n", s); fflush (stderr);
      if (strcmp (s, "{\"b\": 2.5}") != 0)
        {
          fprintf (stderr, "thread2 disturbed by thread1, produced %s\n", s); fflush (stderr);
          abort ();
        }
      free (s);
    }

  /*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
thread2: {"b": 2.5}
thread2: {"b": 2.5}
...
thread2: {"b": 2.5}
thread2: {"b": 2.5}
thread2: {"b": 2,5.0}
thread2 disturbed by thread1, produced {"b": 2,5.0}
Aborted (core dumped)

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

bhaible commented 4 months ago

The cause is that the conversion from double to string, in strconv.c: jsonp_dtostr, 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 thread2, the snprintf call produces "2,5". Then, in the from_locale(buffer) call, it can happen that the return value of localeconv() is disturbed by thread1, such that localeconv()->decimal_point returns ".". In this case, the function from_locale does not change the result, and the comma remains in the result.

bhaible commented 4 months ago

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

akheron commented 4 months ago

Thanks! Fixed in #677.