Make SSLContext's exit() handling more graceful
authorSubodh Iyengar <subodh@fb.com>
Tue, 25 Nov 2014 21:07:16 +0000 (13:07 -0800)
committerDave Watson <davejwatson@fb.com>
Thu, 11 Dec 2014 15:58:48 +0000 (07:58 -0800)
Summary:
Some platforms that SSLContext run on call exit() despite best practices.
This cause the static structures to be destructed and cause race conditions
and crashes.

This new's the static structures so that they won't be destructed on exit()

Test Plan: Unit tests

Reviewed By: seanc@fb.com

Subscribers: trunkagent, ssl-diffs@, seanc, njormrod, folly-diffs@

FB internal diff: D1702186

Tasks: 5572637

Signature: t1:1702186:1416941649:c5bdfd8cc604fc3618f51bdb8b910b5b2cf350ad

folly/io/async/SSLContext.cpp

index 106974b072e5f1bcae861ef763c85b328f4a9d0e..80d414b0ea9bee14d9dcd099e61b0d7460cff6f6 100644 (file)
@@ -538,14 +538,31 @@ struct SSLLock {
   std::mutex mutex;
 };
 
-static std::map<int, SSLContext::SSLLockType> lockTypes;
-static std::unique_ptr<SSLLock[]> locks;
+// Statics are unsafe in environments that call exit().
+// If one thread calls exit() while another thread is
+// references a member of SSLContext, bad things can happen.
+// SSLContext runs in such environments.
+// Instead of declaring a static member we "new" the static
+// member so that it won't be destructed on exit().
+static std::map<int, SSLContext::SSLLockType>* lockTypesInst =
+  new std::map<int, SSLContext::SSLLockType>();
+
+static std::unique_ptr<SSLLock[]>* locksInst =
+  new std::unique_ptr<SSLLock[]>();
+
+static std::unique_ptr<SSLLock[]>& locks() {
+  return *locksInst;
+}
+
+static std::map<int, SSLContext::SSLLockType>& lockTypes() {
+  return *lockTypesInst;
+}
 
 static void callbackLocking(int mode, int n, const char*, int) {
   if (mode & CRYPTO_LOCK) {
-    locks[n].lock();
+    locks()[n].lock();
   } else {
-    locks[n].unlock();
+    locks()[n].unlock();
   }
 }
 
@@ -580,7 +597,7 @@ static void dyn_destroy(struct CRYPTO_dynlock_value* lock, const char*, int) {
 }
 
 void SSLContext::setSSLLockTypes(std::map<int, SSLLockType> inLockTypes) {
-  lockTypes = inLockTypes;
+  lockTypes() = inLockTypes;
 }
 
 void SSLContext::initializeOpenSSL() {
@@ -596,9 +613,9 @@ void SSLContext::initializeOpenSSLLocked() {
   SSL_load_error_strings();
   ERR_load_crypto_strings();
   // static locking
-  locks.reset(new SSLLock[::CRYPTO_num_locks()]);
-  for (auto it: lockTypes) {
-    locks[it.first].lockType = it.second;
+  locks().reset(new SSLLock[::CRYPTO_num_locks()]);
+  for (auto it: lockTypes()) {
+    locks()[it.first].lockType = it.second;
   }
   CRYPTO_set_id_callback(callbackThreadID);
   CRYPTO_set_locking_callback(callbackLocking);
@@ -633,7 +650,7 @@ void SSLContext::cleanupOpenSSLLocked() {
   ERR_free_strings();
   EVP_cleanup();
   ERR_remove_state(0);
-  locks.reset();
+  locks().reset();
   initialized_ = false;
 }