Revert D4389970: add an option to clear error before calling ssl methods

Summary: This reverts commit 12da254d6b281c2b9d522ba19999b2489c0083a2

Differential Revision: D4389970

fbshipit-source-id: 7651425adcf3b86c066d657308af1a0aa6bce5dd

diff --git a/folly/io/async/AsyncSSLSocket.cpp b/folly/io/async/AsyncSSLSocket.cpp
index c19a93263eaf6f5085cab95086f5689d66154425..43f876d84f2dd22be77e19cefebb95bdb478b2e7 100644 (file)
--- a/folly/io/async/AsyncSSLSocket.cpp
+++ b/folly/io/async/AsyncSSLSocket.cpp
@@ -22,8 +22,8 @@
 #include <boost/noncopyable.hpp>
 #include <errno.h>
 #include <fcntl.h>
-#include <openssl/asn1.h>
 #include <openssl/err.h>
+#include <openssl/asn1.h>
 #include <openssl/ssl.h>
 #include <sys/types.h>
 #include <chrono>
@@ -1028,7 +1028,6 @@ AsyncSSLSocket::handleAccept() noexcept {
     SSL_set_msg_callback_arg(ssl_, this);
   }
 
-  clearOpenSSLErrors();
   int ret = SSL_accept(ssl_);
   if (ret <= 0) {
     int sslError;
@@ -1078,18 +1077,6 @@ AsyncSSLSocket::handleAccept() noexcept {
   AsyncSocket::handleInitialReadWrite();
 }
 
-void AsyncSSLSocket::clearOpenSSLErrors() {
-  // Normally clearing out the error before calling into an openssl method
-  // is a bad idea. However there might be other code that we don't control
-  // calling into openssl in the same thread, which doesn't use openssl
-  // correctly. We want to safe-guard ourselves from that code.
-  // However touching the ERR stack each and every time has a cost of taking
-  // a lock, so we only do this when we've opted in.
-  if (clearOpenSSLErrors_) {
-    ERR_clear_error();
-  }
-}
-
 void
 AsyncSSLSocket::handleConnect() noexcept {
   VLOG(3) <<  "AsyncSSLSocket::handleConnect() this=" << this
@@ -1105,7 +1092,6 @@ AsyncSSLSocket::handleConnect() noexcept {
       sslState_ == STATE_CONNECTING);
   assert(ssl_);
 
-  clearOpenSSLErrors();
   auto originalState = state_;
   int ret = SSL_connect(ssl_);
   if (ret <= 0) {
@@ -1273,7 +1259,6 @@ AsyncSSLSocket::performRead(void** buf, size_t* buflen, size_t* offset) {
     return AsyncSocket::performRead(buf, buflen, offset);
   }
 
-  clearOpenSSLErrors();
   int bytes = 0;
   if (!isBufferMovable_) {
     bytes = SSL_read(ssl_, *buf, int(*buflen));

diff --git a/folly/io/async/AsyncSSLSocket.h b/folly/io/async/AsyncSSLSocket.h
index e9aca826c379cb788578460c6179f709b827aa6f..4b0f13f6c1c0dbd90dafba3258d4c49f136b5a5d 100644 (file)
--- a/folly/io/async/AsyncSSLSocket.h
+++ b/folly/io/async/AsyncSSLSocket.h
@@ -664,20 +664,9 @@ class AsyncSSLSocket : public virtual AsyncSocket {
     return sessionResumptionAttempted_;
   }
 
-  /**
-   * Clears the ERR stack before invoking SSL methods.
-   * This is useful if unrelated code that runs in the same thread
-   * does not properly handle SSL error conditions, in which case
-   * it could cause SSL_* methods to fail with incorrect error codes.
-   */
-  void setClearOpenSSLErrors(bool clearErr) {
-    clearOpenSSLErrors_ = clearErr;
-  }
-
  private:
 
   void init();
-  void clearOpenSSLErrors();
 
  protected:
 
@@ -722,7 +711,6 @@ class AsyncSSLSocket : public virtual AsyncSocket {
 
   // This virtual wrapper around SSL_write exists solely for testing/mockability
   virtual int sslWriteImpl(SSL *ssl, const void *buf, int n) {
-    clearOpenSSLErrors();
     return SSL_write(ssl, buf, n);
   }
 
@@ -830,9 +818,6 @@ class AsyncSSLSocket : public virtual AsyncSocket {
   bool sessionResumptionAttempted_{false};
 
   std::unique_ptr<IOBuf> preReceivedData_;
-  // Whether or not to clear the err stack before invocation of another
-  // SSL method
-  bool clearOpenSSLErrors_{false};
 };
 
 } // namespace