Replace LockingPointer::reset with resetWithoutLocking

This required the GnuTLS and void session pointer definitions being made
to use LockingPointer.

Also, use std::move assignment instead of X.reset(Y.release()) pattern.

diff --git a/src/anyp/PortCfg.cc b/src/anyp/PortCfg.cc
index dfcc2dd9de8c50ce8e9c0d9dc622e003b5209eae..2f39f00255be1eb90baf75819658e1cddb4b98ed 100644 (file)
--- a/src/anyp/PortCfg.cc
+++ b/src/anyp/PortCfg.cc
@@ -143,7 +143,7 @@ AnyP::PortCfg::configureSslServerContext()
         }
     }
 
-    secure.staticContext.reset(secure.createStaticServerContext(*this));
+    secure.staticContext.resetWithoutLocking(secure.createStaticServerContext(*this));
     if (!secure.staticContext) {
         char buf[128];
         fatalf("%s_port %s initialization error", AnyP::ProtocolType_str[transport.protocol], s.toUrl(buf, sizeof(buf)));

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 87e9aa7c88a379ad666174bce1b2bd55705d3391..638202d7ca01a4c163f52ea076c9ddf7c48dd1fa 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -177,7 +177,7 @@ ClientHttpRequest::ClientHttpRequest(ConnStateData * aConn) :
 #if USE_OPENSSL
         if (aConn->clientConnection != NULL && aConn->clientConnection->isOpen()) {
             if (auto ssl = fd_table[aConn->clientConnection->fd].ssl.get())
-                al->cache.sslClientCert.reset(SSL_get_peer_certificate(ssl));
+                al->cache.sslClientCert.resetWithoutLocking(SSL_get_peer_certificate(ssl));
         }
 #endif
     }

diff --git a/src/comm.cc b/src/comm.cc
index 4ca20ffac017bf40077143ef33b3703c734d0170..e6062247406d24249d139b99a8bd90cc78a0960f 100644 (file)
--- a/src/comm.cc
+++ b/src/comm.cc
@@ -838,7 +838,7 @@ void
 comm_close_complete(const FdeCbParams &params)
 {
     fde *F = &fd_table[params.fd];
-    F->ssl.reset(nullptr);
+    F->ssl.resetWithoutLocking(nullptr);
 
 #if USE_OPENSSL
     if (F->dynamicSslContext) {

diff --git a/src/fde.h b/src/fde.h
index 35f5387d1f684c98775ea35de3cb8b35ad65b583..4978b5e377cf6f0b25d256224bb3d9752a41ffba 100644 (file)
--- a/src/fde.h
+++ b/src/fde.h
@@ -167,7 +167,7 @@ public:
         halfClosedReader = NULL;
         read_method = NULL;
         write_method = NULL;
-        ssl.reset(nullptr);
+        ssl.resetWithoutLocking(nullptr);
         dynamicSslContext = NULL;
 #if _SQUID_WINDOWS_
         win32.handle = (long)NULL;

diff --git a/src/security/LockingPointer.h b/src/security/LockingPointer.h
index ddb14da90b144a1f77c8d3a39e5529b89fa26347..7e736211e8772ce56c6be286a120d5c7e40e166e 100644 (file)
--- a/src/security/LockingPointer.h
+++ b/src/security/LockingPointer.h
@@ -39,10 +39,10 @@ namespace Security
  * absorption, locking, and unlocking implementations. The API largely
  * follows std::shared_ptr.
  *
- * The constructor and the reset() method import a raw Object pointer.
+ * The constructor and the resetWithoutLocking() method import a raw Object pointer.
  * Normally, reset() would lock(), but libraries like OpenSSL
  * pre-lock objects before they are fed to LockingPointer, necessitating
- * this customization hook.
+ * this resetWithoutLocking() customization hook.
  *
  * The lock() method increments Object's reference counter.
  *
@@ -78,7 +78,7 @@ public:
     explicit LockingPointer(SelfType &&) = default;
     SelfType &operator =(SelfType &&o) {
         if (o.get() != raw)
-            reset(o.release());
+            resetWithoutLocking(o.release());
         return *this;
     }
 
@@ -89,14 +89,14 @@ public:
     T *get() const { return raw; }
 
     /// Reset raw pointer - unlock any previous one and save new one without locking.
-    void reset(T *t) {
+    void resetWithoutLocking(T *t) {
         unlock();
         raw = t;
     }
 
     void resetAndLock(T *t) {
         if (t != get()) {
-            reset(t);
+            resetWithoutLocking(t);
             lock(t);
         }
     }

diff --git a/src/security/ServerOptions.cc b/src/security/ServerOptions.cc
index c6be66a8fc8fe0cd2a9428b1f4d6d431d9413106..fe37177de262f2775a644161ecd49e818e59b37a 100644 (file)
--- a/src/security/ServerOptions.cc
+++ b/src/security/ServerOptions.cc
@@ -159,7 +159,7 @@ Security::ServerOptions::loadDhParams()
         }
     }
 
-    parsedDhParams.reset(dhp);
+    parsedDhParams.resetWithoutLocking(dhp);
 #endif
 }
 

diff --git a/src/security/Session.h b/src/security/Session.h
index ae2a8ba0fd771f66746a67dfa958c1d31120dcb8..fb009df20f15bbef5502667ebe0a74f67d2719de 100644 (file)
--- a/src/security/Session.h
+++ b/src/security/Session.h
@@ -38,13 +38,15 @@ typedef gnutls_session_t SessionPtr;
 // Locks can be implemented attaching locks counter to gnutls_session_t
 // objects using the gnutls_session_set_ptr()/gnutls_session_get_ptr ()
 // library functions
-typedef std::unique_ptr<struct gnutls_session_int, std::function<decltype(gnutls_deinit)>> SessionPointer;
+//typedef std::unique_ptr<struct gnutls_session_int, std::function<decltype(gnutls_deinit)>> SessionPointer;
+CtoCpp1(gnutls_deinit, gnutls_session_t);
+typedef LockingPointer<struct gnutls_session_int, gnutls_deinit_cpp, -1> SessionPointer;
 
 #else
 // use void* so we can check against NULL
 typedef void* SessionPtr;
-// use nullptr_t so default_delete works
-typedef std::unique_ptr<std::nullptr_t> SessionPointer;
+CtoCpp1(xfree, SessionPtr);
+typedef LockingPointer<void, xfree_cpp, -1> SessionPointer;
 
 #endif
 

diff --git a/src/security/cert_generators/file/certificate_db.cc b/src/security/cert_generators/file/certificate_db.cc
index b91512402da3f4a6e64c8c0c10b881db61dd03c3..89ec3238281fd392dea0a4c823eeeb7042209eac 100644 (file)
--- a/src/security/cert_generators/file/certificate_db.cc
+++ b/src/security/cert_generators/file/certificate_db.cc
@@ -310,8 +310,8 @@ bool Ssl::CertificateDb::addCertAndPrivateKey(Security::CertPointer & cert, Ssl:
         Ssl::EVP_PKEY_Pointer findPkey;
         if (pure_find(useName.empty() ? subject.get() : useName, findCert, findPkey)) {
             // Replace with database certificate
-            cert.reset(findCert.release());
-            pkey.reset(findPkey.release());
+            cert = std::move(findCert);
+            pkey = std::move(findPkey);
             return true;
         }
         // pure_find may fail because the entry is expired, or because the

diff --git a/src/security/cert_generators/file/security_file_certgen.cc b/src/security/cert_generators/file/security_file_certgen.cc
index ab9f9a9df5de784c4b5449a42d48827933df1cd6..63e96c889deda803e6385410f5e1b1a36f317f97 100644 (file)
--- a/src/security/cert_generators/file/security_file_certgen.cc
+++ b/src/security/cert_generators/file/security_file_certgen.cc
@@ -204,8 +204,8 @@ static bool processNewRequest(Ssl::CrtdMessage & request_message, std::string co
         if (!Ssl::certificateMatchesProperties(cert.get(), certProperties)) {
             // The certificate changed (renewed or other reason).
             // Generete a new one with the updated fields.
-            cert.reset(NULL);
-            pkey.reset(NULL);
+            cert.resetWithoutLocking(nullptr);
+            pkey.resetWithoutLocking(nullptr);
             db.purgeCert(cert_subject);
         }
     }

diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc
index a2db2e16ff45096393fde15d1ba881541d993d62..dec4a337f8d7a3fef42b580933176a64f8b1e2ff 100644 (file)
--- a/src/ssl/PeekingPeerConnector.cc
+++ b/src/ssl/PeekingPeerConnector.cc
@@ -339,7 +339,7 @@ Ssl::PeekingPeerConnector::handleServerCertificate()
 
         // remember the server certificate for later use
         if (Ssl::ServerBump *serverBump = csd->serverBump()) {
-            serverBump->serverCert.reset(serverCert.release());
+            serverBump->serverCert = std::move(serverCert);
         }
     }
 }
@@ -354,7 +354,7 @@ Ssl::PeekingPeerConnector::serverCertificateVerified()
         else {
             const int fd = serverConnection()->fd;
             Security::SessionPtr ssl = fd_table[fd].ssl.get();
-            serverCert.reset(SSL_get_peer_certificate(ssl));
+            serverCert.resetWithoutLocking(SSL_get_peer_certificate(ssl));
         }
         if (serverCert.get()) {
             csd->resetSslCommonName(Ssl::CommonHostName(serverCert.get()));

diff --git a/src/ssl/gadgets.cc b/src/ssl/gadgets.cc
index 9d594be572fb372fd3957d08465450d505d4e660..3fc5f5d6559cbef88a0d3fd8dde16af21621e8e9 100644 (file)
--- a/src/ssl/gadgets.cc
+++ b/src/ssl/gadgets.cc
@@ -130,12 +130,12 @@ bool Ssl::readCertAndPrivateKeyFromMemory(Security::CertPointer & cert, Ssl::EVP
     BIO_puts(bio.get(), bufferToRead);
 
     X509 * certPtr = NULL;
-    cert.reset(PEM_read_bio_X509(bio.get(), &certPtr, 0, 0));
+    cert.resetWithoutLocking(PEM_read_bio_X509(bio.get(), &certPtr, 0, 0));
     if (!cert)
         return false;
 
     EVP_PKEY * pkeyPtr = NULL;
-    pkey.reset(PEM_read_bio_PrivateKey(bio.get(), &pkeyPtr, 0, 0));
+    pkey.resetWithoutLocking(PEM_read_bio_PrivateKey(bio.get(), &pkeyPtr, 0, 0));
     if (!pkey)
         return false;
 
@@ -148,7 +148,7 @@ bool Ssl::readCertFromMemory(Security::CertPointer & cert, char const * bufferTo
     BIO_puts(bio.get(), bufferToRead);
 
     X509 * certPtr = NULL;
-    cert.reset(PEM_read_bio_X509(bio.get(), &certPtr, 0, 0));
+    cert.resetWithoutLocking(PEM_read_bio_X509(bio.get(), &certPtr, 0, 0));
     if (!cert)
         return false;
 
@@ -511,7 +511,7 @@ static bool generateFakeSslCertificate(Security::CertPointer & certToStore, Ssl:
     if (properties.signWithPkey.get())
         pkey.resetAndLock(properties.signWithPkey.get());
     else // if not exist generate one
-        pkey.reset(Ssl::createSslPrivateKey());
+        pkey.resetWithoutLocking(Ssl::createSslPrivateKey());
 
     if (!pkey)
         return false;
@@ -550,8 +550,8 @@ static bool generateFakeSslCertificate(Security::CertPointer & certToStore, Ssl:
     if (!ret)
         return false;
 
-    certToStore.reset(cert.release());
-    pkeyToStore.reset(pkey.release());
+    certToStore = std::move(cert);
+    pkeyToStore = std::move(pkey);
     return true;
 }
 
@@ -676,11 +676,11 @@ void Ssl::readCertAndPrivateKeyFromFiles(Security::CertPointer & cert, Ssl::EVP_
 {
     if (keyFilename == NULL)
         keyFilename = certFilename;
-    pkey.reset(readSslPrivateKey(keyFilename));
-    cert.reset(readSslX509Certificate(certFilename));
+    pkey.resetWithoutLocking(readSslPrivateKey(keyFilename));
+    cert.resetWithoutLocking(readSslX509Certificate(certFilename));
     if (!pkey || !cert || !X509_check_private_key(cert.get(), pkey.get())) {
-        pkey.reset(NULL);
-        cert.reset(NULL);
+        pkey.resetWithoutLocking(nullptr);
+        cert.resetWithoutLocking(nullptr);
     }
 }
 

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 7f9e569ec5513420a0036cbd06bcbf0c5a9069d4..be9f84dea3c63706ea06880bcc14115b91490d17 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -315,7 +315,7 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
                 }
                 delete filledCheck->sslErrors;
                 filledCheck->sslErrors = NULL;
-                filledCheck->serverCert.reset(NULL);
+                filledCheck->serverCert.resetWithoutLocking(nullptr);
             }
             // If the certificate validator is used then we need to allow all errors and
             // pass them to certficate validator for more processing
@@ -1273,11 +1273,11 @@ void Ssl::readCertChainAndPrivateKeyFromFiles(Security::CertPointer & cert, EVP_
     // XXX: ssl_ask_password_cb needs SSL_CTX_set_default_passwd_cb_userdata()
     // so this may not fully work iff Config.Program.ssl_password is set.
     pem_password_cb *cb = ::Config.Program.ssl_password ? &ssl_ask_password_cb : NULL;
-    pkey.reset(readSslPrivateKey(keyFilename, cb));
-    cert.reset(readSslX509CertificatesChain(certFilename, chain.get()));
+    pkey.resetWithoutLocking(readSslPrivateKey(keyFilename, cb));
+    cert.resetWithoutLocking(readSslX509CertificatesChain(certFilename, chain.get()));
     if (!pkey || !cert || !X509_check_private_key(cert.get(), pkey.get())) {
-        pkey.reset(NULL);
-        cert.reset(NULL);
+        pkey.resetWithoutLocking(nullptr);
+        cert.resetWithoutLocking(nullptr);
     }
 }
 
@@ -1319,7 +1319,7 @@ SslCreate(Security::ContextPtr sslContext, const int fd, Ssl::Bio::Type type, co
         if (BIO *bio = Ssl::Bio::Create(fd, type)) {
             Ssl::Bio::Link(ssl, bio); // cannot fail
 
-            fd_table[fd].ssl.reset(ssl);
+            fd_table[fd].ssl.resetWithoutLocking(ssl);
             fd_table[fd].read_method = &ssl_read_method;
             fd_table[fd].write_method = &ssl_write_method;
             fd_note(fd, squidCtx);