Skip to content
This repository

Added a utility to set the ssl library's MT hooks. #527

Merged
merged 1 commit into from over 2 years ago

5 participants

Jim Carroll wsoltys Joakim Plate arnova davilla
Jim Carroll
Collaborator

This should fix the MT problems with libraries that use SSL. Namely, libcurl. It sets openssl and/or gcrypt locking hooks. It expects gcrypt to be used only on LINUX like machines.

Currently the Windows implementation is missing (at least) the openssl header file(s).

@WiSo, can you let me know what I need to do to get openssl/crypto.h installed on windows? We apparently already have the dll.

Jim Carroll
Collaborator

WiSo, sorry about the miss-ping (if you check).

@wsoltys is who I meant. ^^

wsoltys
Collaborator

hehe, WiSo = wsoltys so you pinged me twice ;)
We're using a precompiled libcurl from here http://www.gknw.de/mirror/curl/win32/old_releases/. Problem is every third party lib can ship it's own version of ssl. If we just have an eye on curl we can take the dev package of the version used by libcurl and put it in our dependency lib. We just have to take this version in future too if we need ssl somewhere else and want to use our dependency system (or bump it if we bump libcurl).
Let me see if I can cook something together today or tomorrow.

wsoltys
Collaborator

I've added the missing headers but getting unresolved external symbols for _CRYPTO_set_locking_callback and _CRYPTO_set_id_callback. To resolve them I would need the ssl import lib which fits to the ssl dll used by the third party build of libcurl which we don't have.
Do we have the curl locking in windows as well or can we skip this implementation?

wsoltys
Collaborator

so we don't need any changes on win32. If worth I can change to a newer version of libcurl which uses openssl 1.0.0

Jim Carroll
Collaborator

If that's the case I'm going to simply add a destructor to clean up the locks then check this in. Is everyone Ok with that?

Deleted user

The content you are editing has changed. Reload the page and try again.

what a bloody mess. pull is fine with me.

Sending Request…

Attach images by dragging & dropping or selecting them. Octocat-spinner-32 Uploading your images… Unfortunately, we don't support that file type. Try again with a PNG, GIF, or JPG. Yowza, that's a big file. Try again with an image file smaller than 10MB. This browser doesn't support image attachments. We recommend updating to the latest Internet Explorer, Google Chrome, or Firefox. Something went really wrong, and we can't process that image. Try again.

Joakim Plate
Collaborator

We should do as done in that ffmpeg commit, only set the thread id for older versions.

arnova
Collaborator

I'm fine with this. Are you sure this works properly with libcurl-gnutls? I'm not that familar with the internals of this...

Jim Carroll
Collaborator

@arnova, I copied the code from an example and on my system HAVE_GCRYPT is true and it executes. I'm not sure how else to verify it since it's used indirectly anyway.

davilla
Collaborator

status on this pull/req ? I've been seeing reports of hangs and bt logs that point to

libcrypto.0.9.8.dylib 0x0276a6b8 lh_insert + 152

Jim Carroll
Collaborator

I have a code change that includes a simplification and cleanup (previous to this the locks don't get cleaned up) I didn't get a chance to check in this weekend. I'll do it shortly.

Jim Carroll
Collaborator

Ok, one more review would help.

Jim Carroll
Collaborator

Ok, I'm putting this in. Please let me know if it addresses the problems we've been seeing.

Jim Carroll jimfcarroll merged commit f587ca9 into from November 15, 2011
Jim Carroll jimfcarroll closed this November 15, 2011
davilla davilla referenced this pull request from a commit November 16, 2011
[osx/ios] sync xcode projects to pull request #527 57ba0cb
Tobias Hieta tru referenced this pull request from a commit in plexinc/plex-home-theater-public June 18, 2013
Tobias Hieta Fix playing from remote
Fixes #527, Fixes #521
c46f820
notspiff notspiff referenced this pull request from a commit in notspiff/xbmc-cmake December 05, 2011
[osx] fixed, add missing bit from xbmc/xbmc#527 e698e3e
notspiff notspiff referenced this pull request from a commit in notspiff/xbmc-cmake December 05, 2011
[ios] fixed, add missing bit from xbmc/xbmc#527 eb9e268
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Nov 15, 2011
Added a utility to set the ssl library's MT hooks. 8541cc3
This page is out of date. Refresh to see the latest.
12  configure.in
@@ -698,6 +698,18 @@ AC_CHECK_HEADER([vorbis/vorbisenc.h],, AC_MSG_ERROR($missing_library))
698 698
 AC_CHECK_HEADER([libmodplug/modplug.h],, AC_MSG_ERROR($missing_library))
699 699
 AC_CHECK_HEADER([curl/curl.h],, AC_MSG_ERROR($missing_library))
700 700
 AC_CHECK_HEADER([FLAC/stream_decoder.h],, AC_MSG_ERROR($missing_library))
  701
+
  702
+# we need to check for the header because if it exists we set the openssl
  703
+# and gcrypt MT callback hooks. This is mostly so that libcurl operates 
  704
+# in MT manner correctly.
  705
+AC_CHECK_HEADER([openssl/crypto.h], AC_DEFINE([HAVE_OPENSSL],[1],[Define if we have openssl]),)
  706
+AC_CHECK_HEADER([gcrypt.h], gcrypt_headers_available=yes,gcrypt_headers_available=no)
  707
+if test "$gcrypt_headers_available" = "yes"; then
  708
+   # if we have the headers then we must have the lib
  709
+   AC_CHECK_LIB([gcrypt],[gcry_control],, AC_MSG_ERROR($missing_library))
  710
+   AC_DEFINE([HAVE_GCRYPT],[1],[Define if we have gcrypt])
  711
+fi
  712
+
701 713
 AC_CHECK_LIB([bz2],         [main],, AC_MSG_ERROR($missing_library))
702 714
 AC_CHECK_LIB([jpeg],        [main],, AC_MSG_ERROR($missing_library)) # check for cximage
703 715
 AC_CHECK_LIB([tiff],        [main],, AC_MSG_ERROR($missing_library))
124  xbmc/utils/CryptThreading.cpp
... ...
@@ -0,0 +1,124 @@
  1
+/*
  2
+ *      Copyright (C) 2005-2008 Team XBMC
  3
+ *      http://www.xbmc.org
  4
+ *
  5
+ *  This Program is free software; you can redistribute it and/or modify
  6
+ *  it under the terms of the GNU General Public License as published by
  7
+ *  the Free Software Foundation; either version 2, or (at your option)
  8
+ *  any later version.
  9
+ *
  10
+ *  This Program is distributed in the hope that it will be useful,
  11
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  12
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  13
+ *  GNU General Public License for more details.
  14
+ *
  15
+ *  You should have received a copy of the GNU General Public License
  16
+ *  along with XBMC; see the file COPYING.  If not, write to
  17
+ *  the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
  18
+ *  http://www.gnu.org/copyleft/gpl.html
  19
+ *
  20
+ */
  21
+
  22
+#ifdef TARGET_WINDOWS
  23
+#error "The threading options for the cryptography libraries don't need to be and shouldn't be set on Windows. Do not include CryptThreading in your windows project."
  24
+#endif
  25
+
  26
+#include "CryptThreading.h"
  27
+#include "threads/Thread.h"
  28
+#include "utils/log.h"
  29
+
  30
+#if (defined HAVE_CONFIG_H) && (!defined WIN32)
  31
+  #include "config.h"
  32
+#else
  33
+#define HAVE_OPENSSL
  34
+#endif
  35
+
  36
+#ifdef HAVE_OPENSSL
  37
+#include <openssl/crypto.h>
  38
+#endif
  39
+
  40
+#ifdef HAVE_GCRYPT
  41
+#include <gcrypt.h>
  42
+#include <errno.h>
  43
+
  44
+GCRY_THREAD_OPTION_PTHREAD_IMPL;
  45
+#endif
  46
+
  47
+/* ========================================================================= */
  48
+/* openssl locking implementation for curl */
  49
+static CCriticalSection* getlock(int index)
  50
+{
  51
+  return g_cryptThreadingInitializer.get_lock(index);
  52
+}
  53
+
  54
+static void lock_callback(int mode, int type, const char* file, int line)
  55
+{
  56
+  if (mode & 0x01 /* CRYPTO_LOCK from openssl/crypto.h */ )
  57
+    getlock(type)->lock();
  58
+  else
  59
+    getlock(type)->unlock();
  60
+}
  61
+
  62
+static unsigned long thread_id()
  63
+{
  64
+  return (unsigned long)CThread::GetCurrentThreadId();
  65
+}
  66
+/* ========================================================================= */
  67
+
  68
+CryptThreadingInitializer::CryptThreadingInitializer()
  69
+{
  70
+  bool attemptedToSetSSLMTHook = false;
  71
+#ifdef HAVE_OPENSSL
  72
+  // set up OpenSSL
  73
+  numlocks = CRYPTO_num_locks();
  74
+  CRYPTO_set_id_callback(thread_id);
  75
+  CRYPTO_set_locking_callback(lock_callback);
  76
+  attemptedToSetSSLMTHook = true;
  77
+#else
  78
+  numlocks = 1;
  79
+#endif
  80
+
  81
+  locks = new CCriticalSection*[numlocks];
  82
+  for (int i = 0; i < numlocks; i++)
  83
+    locks[i] = NULL;
  84
+
  85
+#ifdef HAVE_GCRYPT
  86
+  // set up gcrypt
  87
+  gcry_control(GCRYCTL_SET_THREAD_CBS, &gcry_threads_pthread);
  88
+  attemptedToSetSSLMTHook = true;
  89
+#endif
  90
+
  91
+  if (!attemptedToSetSSLMTHook)
  92
+    CLog::Log(LOGWARNING, "Could not determine the libcurl security library to set the locking scheme. This may cause problem with multithreaded use of ssl or libraries that depend on it (libcurl).");
  93
+  
  94
+}
  95
+
  96
+CryptThreadingInitializer::~CryptThreadingInitializer()
  97
+{
  98
+  CSingleLock l(locksLock);
  99
+#ifdef HAVE_OPENSSL
  100
+  CRYPTO_set_locking_callback(NULL);
  101
+#endif
  102
+
  103
+  for (int i = 0; i < numlocks; i++)
  104
+    delete locks[i]; // I always forget ... delete is NULL safe.
  105
+
  106
+  delete [] locks;
  107
+}
  108
+
  109
+CCriticalSection* CryptThreadingInitializer::get_lock(int index)
  110
+{
  111
+  CSingleLock l(locksLock);
  112
+  CCriticalSection* curlock = locks[index];
  113
+  if (curlock == NULL)
  114
+  {
  115
+    curlock = new CCriticalSection();
  116
+    locks[index] = curlock;
  117
+  }
  118
+
  119
+  return curlock;
  120
+}
  121
+
  122
+
  123
+
  124
+
41  xbmc/utils/CryptThreading.h
... ...
@@ -0,0 +1,41 @@
  1
+/*
  2
+ *      Copyright (C) 2005-2008 Team XBMC
  3
+ *      http://www.xbmc.org
  4
+ *
  5
+ *  This Program is free software; you can redistribute it and/or modify
  6
+ *  it under the terms of the GNU General Public License as published by
  7
+ *  the Free Software Foundation; either version 2, or (at your option)
  8
+ *  any later version.
  9
+ *
  10
+ *  This Program is distributed in the hope that it will be useful,
  11
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  12
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  13
+ *  GNU General Public License for more details.
  14
+ *
  15
+ *  You should have received a copy of the GNU General Public License
  16
+ *  along with XBMC; see the file COPYING.  If not, write to
  17
+ *  the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
  18
+ *  http://www.gnu.org/copyleft/gpl.html
  19
+ *
  20
+ */
  21
+
  22
+#pragma once
  23
+
  24
+#include "utils/GlobalsHandling.h"
  25
+#include "threads/CriticalSection.h"
  26
+
  27
+class CryptThreadingInitializer
  28
+{
  29
+  CCriticalSection** locks;
  30
+  int numlocks;
  31
+  CCriticalSection locksLock;
  32
+
  33
+public:
  34
+  CryptThreadingInitializer();
  35
+  ~CryptThreadingInitializer();
  36
+
  37
+  CCriticalSection* get_lock(int index);
  38
+};
  39
+
  40
+XBMC_GLOBAL_REF(CryptThreadingInitializer,g_cryptThreadingInitializer);
  41
+#define g_cryptThreadingInitializer XBMC_GLOBAL_USE(CryptThreadingInitializer)
1  xbmc/utils/Makefile
@@ -7,6 +7,7 @@ SRCS=AlarmClock.cpp \
7 7
      CharsetConverter.cpp \
8 8
      CPUInfo.cpp \
9 9
      Crc32.cpp \
  10
+     CryptThreading.cpp \
10 11
      DownloadQueue.cpp \
11 12
      DownloadQueueManager.cpp \
12 13
      Fanart.cpp \
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.