]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
dnsdist: Wrap pthread_ objects
authorRemi Gacogne <remi.gacogne@powerdns.com>
Fri, 24 Apr 2020 15:27:50 +0000 (17:27 +0200)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Tue, 28 Apr 2020 13:39:35 +0000 (15:39 +0200)
13 files changed:
pdns/dnscrypt.cc
pdns/dnscrypt.hh
pdns/dnsdist-cache.hh
pdns/dnsdist.hh
pdns/dnsdistdist/dnsdist-backend.cc
pdns/dnsdistdist/dnsdist-kvs.cc
pdns/dnsdistdist/dnsdist-kvs.hh
pdns/dnsdistdist/dnsdist-rules.hh
pdns/dnsdistdist/libssl.cc
pdns/dnsdistdist/tcpiohandler.cc
pdns/libssl.hh
pdns/lock.hh
pdns/test-dnsdistpacketcache_cc.cc

index db096659decac08d5b5c44a10d32b0f7b14c91f3..17eb99666fba4d7d13c309c92bf0f8bc41d1d8a3 100644 (file)
@@ -25,7 +25,6 @@
 #include "dolog.hh"
 #include "dnscrypt.hh"
 #include "dnswriter.hh"
-#include "lock.hh"
 
 DNSCryptPrivateKey::DNSCryptPrivateKey()
 {
@@ -125,20 +124,15 @@ DNSCryptQuery::~DNSCryptQuery()
 
 
 DNSCryptContext::~DNSCryptContext() {
-  pthread_rwlock_destroy(&d_lock);
 }
 
 DNSCryptContext::DNSCryptContext(const std::string& pName, const std::vector<CertKeyPaths>& certKeys): d_certKeyPaths(certKeys), providerName(pName)
 {
-  pthread_rwlock_init(&d_lock, 0);
-
   reloadCertificates();
 }
 
 DNSCryptContext::DNSCryptContext(const std::string& pName, const DNSCryptCert& certificate, const DNSCryptPrivateKey& pKey): providerName(pName)
 {
-  pthread_rwlock_init(&d_lock, 0);
-
   addNewCertificate(certificate, pKey);
 }
 
index 53b09f5fadf52c7b7b0baaed9a8ed4ab0ba2f44b..a010ebdc5a7a99bf64aed7ec37b5f02ef0222dae 100644 (file)
@@ -51,6 +51,7 @@ private:
 #include <sodium.h>
 
 #include "dnsname.hh"
+#include "lock.hh"
 
 #define DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE (crypto_sign_ed25519_PUBLICKEYBYTES)
 #define DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE (crypto_sign_ed25519_SECRETKEYBYTES)
@@ -285,7 +286,7 @@ private:
 
   void addNewCertificate(std::shared_ptr<DNSCryptCertificatePair>& newCert, bool reload=false);
 
-  pthread_rwlock_t d_lock;
+  ReadWriteLock d_lock;
   std::vector<std::shared_ptr<DNSCryptCertificatePair>> d_certs;
   std::vector<CertKeyPaths> d_certKeyPaths;
   DNSName providerName;
index 4b03ad4151aede5e3e748a6a47e90b36c79ce753..966f55cf50fa757ec58063a330078a43e5305c0e 100644 (file)
@@ -92,22 +92,18 @@ private:
   public:
     CacheShard(): d_entriesCount(0)
     {
-      pthread_rwlock_init(&d_lock, nullptr);
     }
     CacheShard(const CacheShard& old): d_entriesCount(0)
     {
-      pthread_rwlock_init(&d_lock, nullptr);
-    }
-    ~CacheShard() {
-      pthread_rwlock_destroy(&d_lock);
     }
+
     void setSize(size_t maxSize)
     {
       d_map.reserve(maxSize);
     }
 
     std::unordered_map<uint32_t,CacheValue> d_map;
-    pthread_rwlock_t d_lock;
+    ReadWriteLock d_lock;
     std::atomic<uint64_t> d_entriesCount;
   };
 
index 3785baffd89e398f576b92523f489da88f36a898..949e5a08c74086bbd5db85b483859dfaed3400da 100644 (file)
@@ -574,15 +574,13 @@ typedef std::function<std::tuple<bool, string>(const DNSQuestion* dq)> QueryCoun
 struct QueryCount {
   QueryCount()
   {
-    pthread_rwlock_init(&queryLock, nullptr);
   }
   ~QueryCount()
   {
-    pthread_rwlock_destroy(&queryLock);
   }
   QueryCountRecords records;
   QueryCountFilter filter;
-  pthread_rwlock_t queryLock;
+  ReadWriteLock queryLock;
   bool enabled{false};
 };
 
@@ -771,11 +769,10 @@ struct DownstreamState
         fd = -1;
       }
     }
-    pthread_rwlock_destroy(&d_lock);
   }
   boost::uuids::uuid id;
   std::vector<unsigned int> hashes;
-  mutable pthread_rwlock_t d_lock;
+  mutable ReadWriteLock d_lock;
   std::vector<int> sockets;
   const std::string sourceItfName;
   std::mutex socketsLock;
@@ -913,11 +910,9 @@ struct ServerPool
 {
   ServerPool()
   {
-    pthread_rwlock_init(&d_lock, nullptr);
   }
   ~ServerPool()
   {
-    pthread_rwlock_destroy(&d_lock);
   }
 
   const std::shared_ptr<DNSDistPacketCache> getCache() const { return packetCache; };
@@ -997,7 +992,7 @@ struct ServerPool
 
 private:
   ServerPolicy::NumberedServerVector d_servers;
-  pthread_rwlock_t d_lock;
+  ReadWriteLock d_lock;
   bool d_useECS{false};
 };
 
index fbce5dd708350464b63941ef0963cb68ab406d6a..2f7a7dbff822b531cbfd43495f434561d75a0c64 100644 (file)
@@ -137,7 +137,6 @@ void DownstreamState::setWeight(int newWeight)
 
 DownstreamState::DownstreamState(const ComboAddress& remote_, const ComboAddress& sourceAddr_, unsigned int sourceItf_, const std::string& sourceItfName_, size_t numberOfSockets, bool connect=true): sourceItfName(sourceItfName_), remote(remote_), sourceAddr(sourceAddr_), sourceItf(sourceItf_), name(remote_.toStringWithPort()), nameWithAddr(remote_.toStringWithPort())
 {
-  pthread_rwlock_init(&d_lock, nullptr);
   id = getUniqueID();
   threadStarted.clear();
 
index e4cc36b506087f457e72ce4c7be53473313600e2..090a17c2e2e3e95745dee345f210d9ddb1ba21bf 100644 (file)
@@ -117,7 +117,6 @@ bool LMDBKVStore::keyExists(const std::string& key)
 
 CDBKVStore::CDBKVStore(const std::string& fname, time_t refreshDelay): d_fname(fname), d_refreshDelay(refreshDelay)
 {
-  pthread_rwlock_init(&d_lock, nullptr);
   d_refreshing.clear();
 
   time_t now = time(nullptr);
@@ -129,7 +128,6 @@ CDBKVStore::CDBKVStore(const std::string& fname, time_t refreshDelay): d_fname(f
 }
 
 CDBKVStore::~CDBKVStore() {
-  pthread_rwlock_destroy(&d_lock);
 }
 
 bool CDBKVStore::reload(const struct stat& st)
index 63e18fdf5e507e94be14b45bfd79db8911fc190d..997de05a2b086e15358da49c4fba01f9fd29af9f 100644 (file)
@@ -193,7 +193,7 @@ private:
 
   std::unique_ptr<CDB> d_cdb{nullptr};
   std::string d_fname;
-  pthread_rwlock_t d_lock;
+  ReadWriteLock d_lock;
   time_t d_mtime{0};
   time_t d_nextCheck{0};
   time_t d_refreshDelay{0};
index 0474a1e1504b8d3c9acee069d7ee8426b40141ee..202e0b055364ca3f589fa2a2c5eeb5cf9a4f0e4f 100644 (file)
@@ -234,13 +234,9 @@ private:
 public:
   TimedIPSetRule()
   {
-    pthread_rwlock_init(&d_lock4, 0);
-    pthread_rwlock_init(&d_lock6, 0);
   }
   ~TimedIPSetRule()
   {
-    pthread_rwlock_destroy(&d_lock4);
-    pthread_rwlock_destroy(&d_lock6);
   }
   bool matches(const DNSQuestion* dq) const override
   {
@@ -302,7 +298,7 @@ public:
 
   void cleanup()
   {
-    time_t now=time(0);
+    time_t now = time(nullptr);
     {
       WriteLock rl(&d_lock4);
 
@@ -360,8 +356,8 @@ private:
   };
   std::unordered_map<IPv6, time_t, IPv6Hash> d_ip6s;
   std::unordered_map<uint32_t, time_t> d_ip4s;
-  mutable pthread_rwlock_t d_lock4;
-  mutable pthread_rwlock_t d_lock6;
+  mutable ReadWriteLock d_lock4;
+  mutable ReadWriteLock d_lock6;
 };
 
 
index 064c1b46d642ca00539158f4d39ab55ebdfaebc4..fa58e39eec68e0e3491d2f053eaf8b22a460f49a 100644 (file)
@@ -7,6 +7,7 @@
 #include <atomic>
 #include <fstream>
 #include <cstring>
+#include <mutex>
 #include <pthread.h>
 
 #include <openssl/conf.h>
 
 #if (OPENSSL_VERSION_NUMBER < 0x1010000fL || (defined LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x2090100fL)
 /* OpenSSL < 1.1.0 needs support for threading/locking in the calling application. */
-static pthread_mutex_t *openssllocks{nullptr};
+
+#include "lock.hh"
+static std::vector<std::mutex> openssllocks;
 
 extern "C" {
 static void openssl_pthreads_locking_callback(int mode, int type, const char *file, int line)
 {
   if (mode & CRYPTO_LOCK) {
-    pthread_mutex_lock(&(openssllocks[type]));
+    openssllocks.at(type).lock();
 
   } else {
-    pthread_mutex_unlock(&(openssllocks[type]));
+    openssllocks.at(type).unlock();
   }
 }
 
@@ -42,24 +45,15 @@ static unsigned long openssl_pthreads_id_callback()
 
 static void openssl_thread_setup()
 {
-  openssllocks = (pthread_mutex_t*)OPENSSL_malloc(CRYPTO_num_locks() * sizeof(pthread_mutex_t));
-
-  for (int i = 0; i < CRYPTO_num_locks(); i++)
-    pthread_mutex_init(&(openssllocks[i]), NULL);
-
-  CRYPTO_set_id_callback(openssl_pthreads_id_callback);
-  CRYPTO_set_locking_callback(openssl_pthreads_locking_callback);
+  openssllocks = std::vector<std::mutex>(CRYPTO_num_locks());
+  CRYPTO_set_id_callback(&openssl_pthreads_id_callback);
+  CRYPTO_set_locking_callback(&openssl_pthreads_locking_callback);
 }
 
 static void openssl_thread_cleanup()
 {
-  CRYPTO_set_locking_callback(NULL);
-
-  for (int i=0; i<CRYPTO_num_locks(); i++) {
-    pthread_mutex_destroy(&(openssllocks[i]));
-  }
-
-  OPENSSL_free(openssllocks);
+  CRYPTO_set_locking_callback(nullptr);
+  openssllocks.clear();
 }
 
 #endif /* (OPENSSL_VERSION_NUMBER < 0x1010000fL || (defined LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < 0x2090100fL) */
@@ -487,13 +481,11 @@ bool libssl_set_min_tls_version(std::unique_ptr<SSL_CTX, void(*)(SSL_CTX*)>& ctx
 
 OpenSSLTLSTicketKeysRing::OpenSSLTLSTicketKeysRing(size_t capacity)
 {
-  pthread_rwlock_init(&d_lock, nullptr);
   d_ticketKeys.set_capacity(capacity);
 }
 
 OpenSSLTLSTicketKeysRing::~OpenSSLTLSTicketKeysRing()
 {
-  pthread_rwlock_destroy(&d_lock);
 }
 
 void OpenSSLTLSTicketKeysRing::addKey(std::shared_ptr<OpenSSLTLSTicketKey> newKey)
index 9823ae569c35f20439c9a5c1aa5988c6641b3a23..b4ea4c96779b7be966dc7ef5e60b7831ec3424a1 100644 (file)
@@ -791,8 +791,6 @@ public:
       throw std::runtime_error("Error setting up TLS cipher preferences to '" + fe.d_tlsConfig.d_ciphers + "' (" + gnutls_strerror(rc) + ") on " + fe.d_addr.toStringWithPort());
     }
 
-    pthread_rwlock_init(&d_lock, nullptr);
-
     try {
       if (fe.d_tlsConfig.d_ticketKeyFile.empty()) {
         handleTicketsKeyRotation(time(nullptr));
@@ -802,15 +800,12 @@ public:
       }
     }
     catch(const std::runtime_error& e) {
-      pthread_rwlock_destroy(&d_lock);
       throw std::runtime_error("Error generating tickets key for TLS context on " + fe.d_addr.toStringWithPort() + ": " + e.what());
     }
   }
 
   virtual ~GnuTLSIOCtx() override
   {
-    pthread_rwlock_destroy(&d_lock);
-
     d_creds.reset();
 
     if (d_priorityCache) {
@@ -876,7 +871,7 @@ private:
   std::unique_ptr<gnutls_certificate_credentials_st, void(*)(gnutls_certificate_credentials_t)> d_creds;
   gnutls_priority_t d_priorityCache{nullptr};
   std::shared_ptr<GnuTLSTicketsKey> d_ticketsKey{nullptr};
-  pthread_rwlock_t d_lock;
+  ReadWriteLock d_lock;
   bool d_enableTickets{true};
 };
 
index b113592555c6a3c5856c73a24de685c743cadf7a..bdca6eaa140b82d40a4969a6f3a05b84aae0fedf 100644 (file)
@@ -94,7 +94,7 @@ public:
 
 private:
   boost::circular_buffer<std::shared_ptr<OpenSSLTLSTicketKey> > d_ticketKeys;
-  pthread_rwlock_t d_lock;
+  ReadWriteLock d_lock;
 };
 
 void* libssl_get_ticket_key_callback_data(SSL* s);
index 7e9a11547b57cd2f14b848e567b77b522421ba7c..d3933059f5e2a5b5d5e3bf7b04b34c85ab8f5367 100644 (file)
@@ -54,6 +54,33 @@ public:
   }
 };
 
+class ReadWriteLock
+{
+public:
+  ReadWriteLock()
+  {
+    if (pthread_rwlock_init(&d_lock, nullptr) != 0) {
+      throw std::runtime_error("Error creating a read-write lock: " + stringerror());
+    }
+  }
+
+  ~ReadWriteLock() {
+    /* might have been moved */
+    pthread_rwlock_destroy(&d_lock);
+  }
+
+  ReadWriteLock(const ReadWriteLock& rhs) = delete;
+  ReadWriteLock& operator=(const ReadWriteLock& rhs) = delete;
+
+  pthread_rwlock_t* getLock()
+  {
+    return &d_lock;
+  }
+
+private:
+  pthread_rwlock_t d_lock;
+};
+
 class WriteLock
 {
   pthread_rwlock_t *d_lock;
@@ -77,6 +104,14 @@ public:
       pthread_rwlock_unlock(d_lock);
   }
 
+  WriteLock(ReadWriteLock& lock): WriteLock(lock.getLock())
+  {
+  }
+
+  WriteLock(ReadWriteLock* lock): WriteLock(lock->getLock())
+  {
+  }
+
   WriteLock(WriteLock&& rhs)
   {
     d_lock = rhs.d_lock;
@@ -118,7 +153,14 @@ public:
     rhs.d_havelock = false;
   }
 
-  
+  TryWriteLock(ReadWriteLock& lock): TryWriteLock(lock.getLock())
+  {
+  }
+
+  TryWriteLock(ReadWriteLock* lock): TryWriteLock(lock->getLock())
+  {
+  }
+
   ~TryWriteLock()
   {
     if(g_singleThreaded)
@@ -157,6 +199,15 @@ public:
     }
     d_havelock=(err==0);
   }
+
+  TryReadLock(ReadWriteLock& lock): TryReadLock(lock.getLock())
+  {
+  }
+
+  TryReadLock(ReadWriteLock* lock): TryReadLock(lock->getLock())
+  {
+  }
+
   TryReadLock(TryReadLock&& rhs)
   {
     d_lock = rhs.d_lock;
@@ -198,6 +249,15 @@ public:
       throw PDNSException("error acquiring rwlock readlock: "+stringerror(err));
     }
   }
+
+  ReadLock(ReadWriteLock& lock): ReadLock(lock.getLock())
+  {
+  }
+
+  ReadLock(ReadWriteLock* lock): ReadLock(lock->getLock())
+  {
+  }
+
   ~ReadLock()
   {
     if(g_singleThreaded)
@@ -209,7 +269,7 @@ public:
   ReadLock(ReadLock&& rhs)
   {
     d_lock = rhs.d_lock;
-    rhs.d_lock=0;
+    rhs.d_lock = nullptr;
   }
   ReadLock(const ReadLock& rhs) = delete;
   ReadLock& operator=(const ReadLock& rhs) = delete;
index 8e41798cb99d13f56045a8692a45d30a41471119..becd1ec2e77e9fe719c0f71465dc2df8ba564218 100644 (file)
@@ -297,14 +297,13 @@ BOOST_AUTO_TEST_CASE(test_PacketCacheNXDomainTTL) {
 
 static DNSDistPacketCache g_PC(500000);
 
-static void *threadMangler(void* off)
+static void threadMangler(unsigned int offset)
 {
   struct timespec queryTime;
   gettime(&queryTime);  // does not have to be accurate ("realTime") in tests
   try {
     ComboAddress remote;
     bool dnssecOK = false;
-    unsigned int offset=(unsigned int)(unsigned long)off;
     for(unsigned int counter=0; counter < 100000; ++counter) {
       DNSName a=DNSName("hello ")+DNSName(std::to_string(counter+offset));
       vector<uint8_t> query;
@@ -337,19 +336,17 @@ static void *threadMangler(void* off)
     cerr<<"Had error: "<<e.reason<<endl;
     throw;
   }
-  return 0;
 }
 
 AtomicCounter g_missing;
 
-static void *threadReader(void* off)
+static void threadReader(unsigned int offset)
 {
   bool dnssecOK = false;
   struct timespec queryTime;
   gettime(&queryTime);  // does not have to be accurate ("realTime") in tests
   try
   {
-    unsigned int offset=(unsigned int)(unsigned long)off;
     vector<DNSResourceRecord> entry;
     ComboAddress remote;
     for(unsigned int counter=0; counter < 100000; ++counter) {
@@ -373,25 +370,31 @@ static void *threadReader(void* off)
     cerr<<"Had error in threadReader: "<<e.reason<<endl;
     throw;
   }
-  return 0;
 }
 
 BOOST_AUTO_TEST_CASE(test_PacketCacheThreaded) {
   try {
-    pthread_t tid[4];
-    for(int i=0; i < 4; ++i)
-      pthread_create(&tid[i], 0, threadMangler, (void*)(i*1000000UL));
-    void* res;
-    for(int i=0; i < 4 ; ++i)
-      pthread_join(tid[i], &res);
+    std::vector<std::thread> threads;
+    for (int i = 0; i < 4; ++i) {
+      threads.push_back(std::thread(threadMangler, i*1000000UL));
+    }
+
+    for (auto& t : threads) {
+      t.join();
+    }
+
+    threads.clear();
 
     BOOST_CHECK_EQUAL(g_PC.getSize() + g_PC.getDeferredInserts() + g_PC.getInsertCollisions(), 400000U);
     BOOST_CHECK_SMALL(1.0*g_PC.getInsertCollisions(), 10000.0);
 
-    for(int i=0; i < 4; ++i)
-      pthread_create(&tid[i], 0, threadReader, (void*)(i*1000000UL));
-    for(int i=0; i < 4 ; ++i)
-      pthread_join(tid[i], &res);
+    for (int i = 0; i < 4; ++i) {
+      threads.push_back(std::thread(threadReader, i*1000000UL));
+    }
+
+    for (auto& t : threads) {
+      t.join();
+    }
 
     BOOST_CHECK((g_PC.getDeferredInserts() + g_PC.getDeferredLookups() + g_PC.getInsertCollisions()) >= g_missing);
   }