From e08365777199901d58e7cf4cc269ffabd07b2659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 1 Nov 2014 10:26:09 +0100 Subject: ssl: clear the OpenSSL locking function We're freeing the memory which holds the locks so we must make sure that the locking function doesn't try to use it. --- src/global.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/global.c b/src/global.c index 1e6bf82f9..2435b5673 100644 --- a/src/global.c +++ b/src/global.c @@ -66,6 +66,7 @@ void openssl_locking_function(int mode, int n, const char *file, int line) static void shutdown_ssl(void) { + CRYPTO_set_locking_callback(NULL); git__free(openssl_locks); } #endif -- cgit v1.2.1 From fe6b51ae40313036436f81d4e87d232691cdacac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sat, 1 Nov 2014 10:45:33 +0100 Subject: ssl: separate locking init from general init Extract the lock-setting functions into their own, as we cannot assume that it's ok for us to set this unconditionally. --- include/git2/threads.h | 16 ++++++++++++++++ src/global.c | 41 +++++++++++++++++++++++------------------ 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/include/git2/threads.h b/include/git2/threads.h index 11f89729a..6b4287033 100644 --- a/include/git2/threads.h +++ b/include/git2/threads.h @@ -44,6 +44,22 @@ GIT_EXTERN(int) git_threads_init(void); */ GIT_EXTERN(void) git_threads_shutdown(void); +/** + * Initialize the OpenSSL locks + * + * OpenSSL requires the application to determine how it performs + * locking. This is a convenience function which libgit2 provides for + * allocating and initializing the locks as well as setting the + * locking function to use the system's native locking functions. + * + * The locking function will be cleared and the memory will be freed + * when you call git_threads_sutdown(). + * + * @return 0 on success, -1 if there are errors or if libgit2 was not + * built with OpenSSL and threading support. + */ +GIT_EXTERN(int) git_openssl_set_locking(void); + /** @} */ GIT_END_DECL #endif diff --git a/src/global.c b/src/global.c index 2435b5673..3e721ee3a 100644 --- a/src/global.c +++ b/src/global.c @@ -64,7 +64,7 @@ void openssl_locking_function(int mode, int n, const char *file, int line) } } -static void shutdown_ssl(void) +static void shutdown_ssl_locking(void) { CRYPTO_set_locking_callback(NULL); git__free(openssl_locks); @@ -96,30 +96,35 @@ static void init_ssl(void) SSL_CTX_free(git__ssl_ctx); git__ssl_ctx = NULL; } +#endif +} +int git_openssl_set_locking(void) +{ +#ifdef GIT_SSL # ifdef GIT_THREADS - { - int num_locks, i; - - num_locks = CRYPTO_num_locks(); - openssl_locks = git__calloc(num_locks, sizeof(git_mutex)); - if (openssl_locks == NULL) { - SSL_CTX_free(git__ssl_ctx); - git__ssl_ctx = NULL; - } + int num_locks, i; - for (i = 0; i < num_locks; i++) { - if (git_mutex_init(&openssl_locks[i]) != 0) { - SSL_CTX_free(git__ssl_ctx); - git__ssl_ctx = NULL; - } - } + num_locks = CRYPTO_num_locks(); + openssl_locks = git__calloc(num_locks, sizeof(git_mutex)); + GITERR_CHECK_ALLOC(openssl_locks); - CRYPTO_set_locking_callback(openssl_locking_function); + for (i = 0; i < num_locks; i++) { + if (git_mutex_init(&openssl_locks[i]) != 0) { + giterr_set(GITERR_SSL, "failed to initialize openssl locks"); + return -1; + } } - git__on_shutdown(shutdown_ssl); + CRYPTO_set_locking_callback(openssl_locking_function); + git__on_shutdown(shutdown_ssl_locking); + return 0; +# else + giterr_set(GITERR_THREAD, "libgit2 as not built with threads"); + return -1; # endif + giterr_set(GITERR_SSL, "libgit2 was not built with OpenSSL support"); + return -1; #endif } -- cgit v1.2.1 From 15bea02c04ebf3172d495f14219b30eba262eb6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 31 Oct 2014 11:24:02 +0100 Subject: docs: explicitly document the threading caveats Talk about sharing objects and error messages; but the most important part is about what to do with the cryptographic libraries, which sadly have to become to responsibility of the application. --- README.md | 5 ++++ THREADING.md | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 THREADING.md diff --git a/README.md b/README.md index 98aaa1dd7..f25bb12bb 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,11 @@ dependencies, it can make use of a few libraries to add to it: - LibSSH2 to enable the SSH transport - iconv (OSX) to handle the HFS+ path encoding peculiarities +Threading +========= + +See [THREADING](THREADING.md) for information + Building libgit2 - Using CMake ============================== diff --git a/THREADING.md b/THREADING.md new file mode 100644 index 000000000..475931885 --- /dev/null +++ b/THREADING.md @@ -0,0 +1,79 @@ +Threads in libgit2 +================== + +You may safely use any libgit2 object from any thread, though there +may be issues depending on the cryptographic libraries libgit2 or its +dependencies link to (more on this later). For libgit2 itself, +provided you take the following into consideration you won't run into +issues: + +Sharing objects +--------------- + +Use an object from a single thread at a time. Most data structures do +not guard against concurrent access themselves. This is because they +are rarely used in isolation and it makes more sense to synchronize +access via a larger lock or similar mechanism. + +There are some objects which are read-only/immutable and are thus safe +to share across threads, such as references and configuration +snapshots. + +Error messages +-------------- + +The error message is thread-local. The `giterr_last()` call must +happen on the same thread as the error in order to get the +message. Often this will be the case regardless, but if you use +something like the GDC on MacOS (where code is executed on an +arbitrary thread), the code must make sure to retrieve the error code +on the thread where the error happened. + +Threads and cryptographic libraries +======================================= + +On Windows +---------- + +When built as a native Windows DLL, libgit2 uses WinCNG and WinHTTP, +both of which are thread-safe. You do not need to do anything special. + +When using libssh2 which itself uses WinCNG, there are no special +steps necessary. If you are using a MinGW or similar environment where +libssh2 uses OpenSSL or libgcrypt, then the non-Windows case affects +you. + +Non-Windows +----------- + +On the rest of the platforms, libgit2 uses OpenSSL to be able to use +HTTPS as a transport. This library is made to be thread-implementation +agnostic, and the users of the library must set which locking function +it should use. This means that libgit2 cannot know what to set as the +user of libgit2 may use OpenSSL independently and the locking settings +must survive libgit2 shutting down. + +libgit2 does provide a convenience function +`git_openssl_set_locking()` to use the platform-native mutex +mechanisms to perform the locking, which you may rely on if you do not +want to use OpenSSL outside of libgit2, or you know that libgit2 will +outlive the rest of the operations. It is not safe to use OpenSSL +multi-threaded after libgit2's shutdown function has been called. + +See the +[OpenSSL documentation](https://www.openssl.org/docs/crypto/threads.html) +on threading for more details. + +libssh2 may be linked against OpenSSL or libgcrypt. If it uses +OpenSSL, you only need to set up threading for OpenSSL once and the +above paragraphs are enough. If it uses libgcrypt, then you need to +set up its locking before using it multi-threaded. libgit2 has no +direct connection to libgcrypt and thus has not convenience functions for +it (but libgcrypt has macros). Read libgcrypt's +[threading documentation for more information](http://www.gnupg.org/documentation/manuals/gcrypt/Multi_002dThreading.html) + +It is your responsibility as an application author or packager to know +what your dependencies are linked against and to take the appropriate +steps to ensure the cryptographic libraries are thread-safe. We agree +that this situation is far from ideal but at this time it is something +the application authors need to deal with. -- cgit v1.2.1