Skip to content
Permalink
Browse files

global_init: Make it thread-safe when possible

- Use compare-and-swap with a full memory barrier to synchronize
  calls to curl_global_init, curl_global_init_mem, curl_global_cleanup
  and curl_global_sslset.

This is a locking method that does not require libcurl to be built with
a threading library. If compare-and-swap with a full memory barrier is
supported by the compiler (gcc, clang) or the OS (Windows) then libcurl
init/cleanup will be locked.

As discused in #3990 curl_global_ functions are not thread-safe which is
a problem for some users of libcurl that are themselves libraries. The
curl_global_ function requirements may be impossible to meet, eg:

"You must not call it when any other thread in the program (i.e. a
thread sharing the same memory) is running. This doesn't just mean no
other thread that is using libcurl. Because curl_global_init calls
functions of other libraries that are similarly thread unsafe, it could
conflict with any other thread that uses these other libraries."

Even if the user builds libcurl to depend only on libraries that offer
documented thread-safe init/cleanup, there is still libcurl's own
init/cleanup where we have a chicken-and-egg problem of how to
initialize and clean up our lock on many platforms. To solve that I've
implemented a lock that has only its state using compare-and-swap and
does not require any initialization or clean up.

We still cannot guarantee curl_global_ functions are thread-safe, but
this change should make it safer for the user.

Ref: #3990
Ref: https://curl.haxx.se/libcurl/c/curl_global_init.html
Ref: ??? probably this has come up before elsewhere

Closes #xxxx
  • Loading branch information...
jay committed Jun 22, 2019
1 parent aae4902 commit ad9e505f97bcf7542f21055b1690d6ef2cc6a9e5
Showing with 232 additions and 34 deletions.
  1. +2 −2 lib/Makefile.inc
  2. +43 −30 lib/easy.c
  3. +114 −0 lib/lazylock.c
  4. +56 −0 lib/lazylock.h
  5. +17 −2 lib/vtls/vtls.c
@@ -59,7 +59,7 @@ LIB_CFILES = file.c timeval.c base64.c hostip.c progress.c formdata.c \
curl_multibyte.c hostcheck.c conncache.c dotdot.c \
x509asn1.c http2.c smb.c curl_endian.c curl_des.c system_win32.c \
mime.c sha256.c setopt.c curl_path.c curl_ctype.c curl_range.c psl.c \
doh.c urlapi.c curl_get_line.c altsvc.c
doh.c urlapi.c curl_get_line.c altsvc.c lazylock.c

LIB_HFILES = arpa_telnet.h netrc.h file.h timeval.h hostip.h progress.h \
formdata.h cookie.h http.h sendf.h ftp.h url.h dict.h if2ip.h \
@@ -80,7 +80,7 @@ LIB_HFILES = arpa_telnet.h netrc.h file.h timeval.h hostip.h progress.h \
x509asn1.h http2.h sigpipe.h smb.h curl_endian.h curl_des.h \
curl_printf.h system_win32.h rand.h mime.h curl_sha256.h setopt.h \
curl_path.h curl_ctype.h curl_range.h psl.h doh.h urlapi-int.h \
curl_get_line.h altsvc.h quic.h
curl_get_line.h altsvc.h quic.h lazylock.h

LIB_RCFILES = libcurl.rc

@@ -76,6 +76,7 @@
#include "setopt.h"
#include "http_digest.h"
#include "system_win32.h"
#include "lazylock.h"

/* The last 3 #include files should be in this order */
#include "curl_printf.h"
@@ -138,22 +139,24 @@ curl_calloc_callback Curl_ccalloc;
* curl_global_init() globally initializes curl given a bitwise set of the
* different features of what to initialize.
*/
static CURLcode global_init(long flags, bool memoryfuncs)
static CURLcode global_init_nolock(long flags,
curl_malloc_callback m,
curl_free_callback f,
curl_realloc_callback r,
curl_strdup_callback s,
curl_calloc_callback c)
{
if(initialized++)
return CURLE_OK;

if(memoryfuncs) {
/* Setup the default memory functions here (again) */
Curl_cmalloc = (curl_malloc_callback)malloc;
Curl_cfree = (curl_free_callback)free;
Curl_crealloc = (curl_realloc_callback)realloc;
Curl_cstrdup = (curl_strdup_callback)system_strdup;
Curl_ccalloc = (curl_calloc_callback)calloc;
Curl_cmalloc = (curl_malloc_callback)m;
Curl_cfree = (curl_free_callback)f;
Curl_crealloc = (curl_realloc_callback)r;
Curl_cstrdup = (curl_strdup_callback)s;
Curl_ccalloc = (curl_calloc_callback)c;
#if defined(WIN32) && defined(UNICODE)
Curl_cwcsdup = (curl_wcsdup_callback)_wcsdup;
Curl_cwcsdup = (curl_wcsdup_callback)_wcsdup;
#endif
}

if(!Curl_ssl_init()) {
DEBUGF(fprintf(stderr, "Error: Curl_ssl_init failed\n"));
@@ -211,14 +214,24 @@ static CURLcode global_init(long flags, bool memoryfuncs)
return CURLE_OK;
}


/**
* curl_global_init() globally initializes curl given a bitwise set of the
* different features of what to initialize.
*/
CURLcode curl_global_init(long flags)
{
return global_init(flags, TRUE);
CURLcode rc;

LOCK_GLOBAL_INIT();
rc = global_init_nolock(flags,
(curl_malloc_callback)malloc,
(curl_free_callback)free,
(curl_realloc_callback)realloc,
(curl_strdup_callback)system_strdup,
(curl_calloc_callback)calloc);
UNLOCK_GLOBAL_INIT();

return rc;
}

/*
@@ -229,35 +242,24 @@ CURLcode curl_global_init_mem(long flags, curl_malloc_callback m,
curl_free_callback f, curl_realloc_callback r,
curl_strdup_callback s, curl_calloc_callback c)
{
CURLcode rc;

/* Invalid input, return immediately */
if(!m || !f || !r || !s || !c)
return CURLE_FAILED_INIT;

if(initialized) {
/* Already initialized, don't do it again, but bump the variable anyway to
work like curl_global_init() and require the same amount of cleanup
calls. */
initialized++;
return CURLE_OK;
}

/* set memory functions before global_init() in case it wants memory
functions */
Curl_cmalloc = m;
Curl_cfree = f;
Curl_cstrdup = s;
Curl_crealloc = r;
Curl_ccalloc = c;
LOCK_GLOBAL_INIT();
rc = global_init_nolock(flags, m, f, r, s, c);
UNLOCK_GLOBAL_INIT();

/* Call the actual init function, but without setting */
return global_init(flags, FALSE);
return rc;
}

/**
* curl_global_cleanup() globally cleanups curl, uses the value of
* "init_flags" to determine what needs to be cleaned up and what doesn't.
*/
void curl_global_cleanup(void)
static void global_cleanup_nolock(void)
{
if(!initialized)
return;
@@ -285,6 +287,17 @@ void curl_global_cleanup(void)
init_flags = 0;
}

/**
* curl_global_cleanup() globally cleanups curl, uses the value of
* "init_flags" to determine what needs to be cleaned up and what doesn't.
*/
void curl_global_cleanup(void)
{
LOCK_GLOBAL_INIT();
global_cleanup_nolock();
UNLOCK_GLOBAL_INIT();
}

/*
* curl_easy_init() is the external interface to alloc, setup and init an
* easy handle that is returned. If anything goes wrong, NULL is returned.
@@ -0,0 +1,114 @@
/***************************************************************************
* _ _ ____ _
* Project ___| | | | _ \| |
* / __| | | | |_) | |
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
* are also available at https://curl.haxx.se/docs/copyright.html.
*
* You may opt to use, copy, modify, merge, publish, distribute and/or sell
* copies of the Software, and permit persons to whom the Software is
* furnished to do so, under the terms of the COPYING file.
*
* This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
* KIND, either express or implied.
*
***************************************************************************/

#include "curl_setup.h"

#include "lazylock.h"
#include <curl/curl.h>
#include "select.h"
#include "system_win32.h"
#include "warnless.h"

/* The last #include files should be: */
#include "curl_memory.h"
#include "memdebug.h"

/*
* Lazy locking for thread synchronization.
*
* This is a type of lock that needs no allocated resources, to avoid the
* chicken-and-egg problem we'd have with thread-safe init/deinit of the lock
* itself during global init/deinit.
*
* The lock is an atomic compare-and-swap with a full memory barrier. Rather
* than aggressive spin to acquire the lock or efficiently wait in a queue it
* will wait to lock by sleeping instead. Acquisition is not ordered.
*
* The lock is not re-entrant since for our purposes locking global init/deinit
* that is not needed. Re-entrancy could be implemented but it would limit
* support to builds that have thread support or some system function to get
* the current thread id.
*
* The implementation of this lock does not (and should not) require libcurl to
* be built with any threading support.
*/

#ifdef HAVE_LAZYLOCK

typedef enum {
LAZYLOCK_UNLOCKED,
/* LAZYLOCK_TRANSIENT, */
LAZYLOCK_LOCKED
} lockstate;

curl_lazylock_obj curl_initlock;

/* atomic compare-and-swap with full memory barrier */
static long compare_and_swap(LAZYLOCK_ATOMIC_TYPENAME long *target,
long oldval, long newval)
{
#ifdef WIN32
return InterlockedCompareExchange(target, newval, oldval);
#elif defined(__clang__)
/* C11's atomic_compare_exchange_strong with memory_order_seq_cst */
__c11_atomic_compare_exchange_strong(target, &oldval, newval,
__ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
return oldval;
#elif defined(__GNUC__)
/* gcc says intel is unclear on whether or not __sync_val_compare_and_swap
gives a full memory barrier so play it safe and call __sync_synchronize.
https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Atomic-Builtins.html */
__sync_synchronize();
return __sync_val_compare_and_swap(target, oldval, newval);
#else
#error "compare-and-swap function not implemented"
#endif
}

void Curl_lazylock_lock(curl_lazylock_obj *lock)
{
int milliseconds = 1;

for(;;) {
long prev = compare_and_swap(lock, LAZYLOCK_UNLOCKED, LAZYLOCK_LOCKED);

/* if the previous state was UNLOCKED then we own LOCKED */
if(prev == LAZYLOCK_UNLOCKED)
return;

Curl_wait_ms(milliseconds);

if(milliseconds < 1024)
milliseconds *= 2;
}
}

void Curl_lazylock_unlock(curl_lazylock_obj *lock)
{
long prev = compare_and_swap(lock, LAZYLOCK_LOCKED, LAZYLOCK_UNLOCKED);
/* the previous state should always be LOCKED */
DEBUGASSERT(prev == LAZYLOCK_LOCKED);
(void)prev;
return;
}

#endif /* HAVE_LAZYLOCK */
@@ -0,0 +1,56 @@
#ifndef HEADER_LAZYLOCK_H
#define HEADER_LAZYLOCK_H
/***************************************************************************
* _ _ ____ _
* Project ___| | | | _ \| |
* / __| | | | |_) | |
* | (__| |_| | _ <| |___
* \___|\___/|_| \_\_____|
*
* Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
* are also available at https://curl.haxx.se/docs/copyright.html.
*
* You may opt to use, copy, modify, merge, publish, distribute and/or sell
* copies of the Software, and permit persons to whom the Software is
* furnished to do so, under the terms of the COPYING file.
*
* This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
* KIND, either express or implied.
*
***************************************************************************/

#include "curl_setup.h"

#if defined(WIN32) || defined(__clang__) || defined(__GNUC__)
#define HAVE_LAZYLOCK
#endif

#ifdef HAVE_LAZYLOCK

/* lazylock object is just the state. type long due to Windows CAS type. */
#ifdef __clang__
#define LAZYLOCK_ATOMIC_TYPENAME _Atomic
#else
#define LAZYLOCK_ATOMIC_TYPENAME
#endif

typedef LAZYLOCK_ATOMIC_TYPENAME long curl_lazylock_obj;

extern curl_lazylock_obj curl_initlock;

/* lazylock is not re-entrant */
void Curl_lazylock_lock(curl_lazylock_obj *lock);
void Curl_lazylock_unlock(curl_lazylock_obj *lock);

#define LOCK_GLOBAL_INIT() Curl_lazylock_lock(&curl_initlock)
#define UNLOCK_GLOBAL_INIT() Curl_lazylock_unlock(&curl_initlock)

#else
#define LOCK_GLOBAL_INIT()
#define UNLOCK_GLOBAL_INIT()
#endif /* HAVE_LAZYLOCK */

#endif /* HEADER_LAZYLOCK_H */
@@ -63,6 +63,7 @@
#include "warnless.h"
#include "curl_base64.h"
#include "curl_printf.h"
#include "lazylock.h"

/* The last #include files should be: */
#include "curl_memory.h"
@@ -1301,8 +1302,9 @@ static int multissl_init(const struct Curl_ssl *backend)
return 0;
}

CURLsslset curl_global_sslset(curl_sslbackend id, const char *name,
const curl_ssl_backend ***avail)
static CURLsslset global_sslset_nolock(curl_sslbackend id,
const char *name,
const curl_ssl_backend ***avail)
{
int i;

@@ -1330,6 +1332,19 @@ CURLsslset curl_global_sslset(curl_sslbackend id, const char *name,
return CURLSSLSET_UNKNOWN_BACKEND;
}

CURLsslset curl_global_sslset(curl_sslbackend id,
const char *name,
const curl_ssl_backend ***avail)
{
CURLsslset rc;

LOCK_GLOBAL_INIT();
rc = global_sslset_nolock(id, name, avail);
UNLOCK_GLOBAL_INIT();

return rc;
}

#else /* USE_SSL */
CURLsslset curl_global_sslset(curl_sslbackend id, const char *name,
const curl_ssl_backend ***avail)

0 comments on commit ad9e505

Please sign in to comment.
You can’t perform that action at this time.