[#1732] Changes after rebasing and TSAN run

src/lib/http/http_messages.h modified: src/lib/http/tests/mt_client_unittests.cc
	- fixed read/write TSAN error

5 files changed, 35 insertions, 21 deletions

diff --git a/src/lib/http/client.cc b/src/lib/http/client.cc
index 96e2506a77..4521ea42c6 100644
--- a/src/lib/http/client.cc
+++ b/src/lib/http/client.cc
@@ -178,15 +178,6 @@ public:
     /// @return true if the premature timeout is suspected, false otherwise.
     bool checkPrematureTimeout(const uint64_t transid);
 
-    /// @brief Fetches the native socket descriptor
-    ///
-    /// This is intended strictly for logging purposes. Manipulating or
-    /// using this fd for anyother purpose is not recommended.
-    ///
-    /// @return the connection's socket file descriptor or -1 if the
-    /// socket is not currently open.
-    int getSocketFd() { return(socket_.getNative()); }
-
 private:
 
     /// @brief Starts new asynchronous transaction (HTTP request and response).
@@ -386,6 +377,18 @@ private:
     /// Connection and ConnectionPool.
     boost::weak_ptr<ConnectionPool> conn_pool_;
 
+    int getSocketFd() {
+        int fd = -1;
+
+        if (tcp_socket_) {
+            fd = tcp_socket_->getNative();
+        } else if (tls_socket_) {
+            fd = tls_socket_->getNative();
+        }
+
+        return (fd);
+    }
+
     /// @brief URL for this connection.
     Url url_;
 
@@ -802,12 +805,13 @@ private:
 
             // Room to make another connection with this destination, so make one.
             TOMS_TRACE_LOG("creating a new connection");
-            connection.reset(new Connection(io_service_, shared_from_this(), url));
+            connection.reset(new Connection(io_service_, tls_context, 
+                                            shared_from_this(), url));
             url_connections->addConnection(connection);
         }
 
         // Use the connection to start the transaction.
-        TOMS_TRACE_LOG("doTransaction using fd:" << connection->getSocketFd());
+        TOMS_TRACE_LOG("doTransaction");
         connection->doTransaction(request, response, request_timeout, request_callback,
                                   connect_callback, handshake_callback, close_callback);
     }
@@ -1202,7 +1206,7 @@ Connection::terminate(const boost::system::error_code& ec,
 void
 Connection::terminateInternal(const boost::system::error_code& ec,
                               const std::string& parsing_error) {
-    TOMS_TRACE_LOG(" on:" << socket_.getNative())
+    TOMS_TRACE_LOG(" on:" << getSocketFd())
     HttpResponsePtr response;
 
     if (isTransactionOngoing()) {
@@ -1316,7 +1320,7 @@ Connection::doHandshake(const uint64_t transid) {
 
 void
 Connection::doSend(const uint64_t transid) {
-    TOMS_TRACE_LOG("transid:" << transid << " on:" << socket_.getNative());
+    TOMS_TRACE_LOG("transid:" << transid << " on:" << getSocketFd());
     SocketCallback socket_cb(std::bind(&Connection::sendCallback,
                                        shared_from_this(),
                                        transid,
@@ -1344,7 +1348,7 @@ Connection::doSend(const uint64_t transid) {
 
 void
 Connection::doReceive(const uint64_t transid) {
-    TOMS_TRACE_LOG("transid:" << transid << " on:" << socket_.getNative());
+    TOMS_TRACE_LOG("transid:" << transid << " on:" << getSocketFd());
     TCPEndpoint endpoint;
     SocketCallback socket_cb(std::bind(&Connection::receiveCallback,
                                        shared_from_this(),
@@ -1378,7 +1382,7 @@ void
 Connection::connectCallback(HttpClient::ConnectHandler connect_callback,
                             const uint64_t transid,
                             const boost::system::error_code& ec) {
-    TOMS_TRACE_LOG("transid:" << transid << " on:" << socket_.getNative());
+    TOMS_TRACE_LOG("transid:" << transid << " on:" << getSocketFd());
     if (checkPrematureTimeout(transid)) {
         return;
     }
@@ -1456,7 +1460,7 @@ void
 Connection::sendCallback(const uint64_t transid,
                          const boost::system::error_code& ec,
                          size_t length) {
-    TOMS_TRACE_LOG("transid:" << transid << " on:" << socket_.getNative());
+    TOMS_TRACE_LOG("transid:" << transid << " on:" << getSocketFd());
     if (checkPrematureTimeout(transid)) {
         return;
     }
@@ -1501,7 +1505,7 @@ void
 Connection::receiveCallback(const uint64_t transid,
                             const boost::system::error_code& ec,
                             size_t length) {
-    TOMS_TRACE_LOG("transid:" << transid << " on:" << socket_.getNative());
+    TOMS_TRACE_LOG("transid:" << transid << " on:" << getSocketFd());
     if (checkPrematureTimeout(transid)) {
         return;
     }
diff --git a/src/lib/http/client.h b/src/lib/http/client.h
index 6e4be230d8..a73a7448de 100644
--- a/src/lib/http/client.h
+++ b/src/lib/http/client.h
@@ -8,6 +8,7 @@
 #define HTTP_CLIENT_H
 
 #include <asiolink/io_service.h>
+#include <asiolink/tls_socket.h>
 #include <exceptions/exceptions.h>
 #include <http/url.h>
 #include <http/request.h>
diff --git a/src/lib/http/http_messages.cc b/src/lib/http/http_messages.cc
index 510b40b8f8..7e2cb62bbc 100644
--- a/src/lib/http/http_messages.cc
+++ b/src/lib/http/http_messages.cc
@@ -12,6 +12,7 @@ extern const isc::log::MessageID HTTP_BAD_CLIENT_REQUEST_RECEIVED = "HTTP_BAD_CL
 extern const isc::log::MessageID HTTP_BAD_CLIENT_REQUEST_RECEIVED_DETAILS = "HTTP_BAD_CLIENT_REQUEST_RECEIVED_DETAILS";
 extern const isc::log::MessageID HTTP_BAD_SERVER_RESPONSE_RECEIVED = "HTTP_BAD_SERVER_RESPONSE_RECEIVED";
 extern const isc::log::MessageID HTTP_BAD_SERVER_RESPONSE_RECEIVED_DETAILS = "HTTP_BAD_SERVER_RESPONSE_RECEIVED_DETAILS";
+extern const isc::log::MessageID HTTP_CLIENT_MT_STARTED = "HTTP_CLIENT_MT_STARTED";
 extern const isc::log::MessageID HTTP_CLIENT_REQUEST_AUTHORIZED = "HTTP_CLIENT_REQUEST_AUTHORIZED";
 extern const isc::log::MessageID HTTP_CLIENT_REQUEST_BAD_AUTH_HEADER = "HTTP_CLIENT_REQUEST_BAD_AUTH_HEADER";
 extern const isc::log::MessageID HTTP_CLIENT_REQUEST_NOT_AUTHORIZED = "HTTP_CLIENT_REQUEST_NOT_AUTHORIZED";
@@ -48,6 +49,7 @@ const char* values[] = {
     "HTTP_BAD_CLIENT_REQUEST_RECEIVED_DETAILS", "detailed information about bad request received from %1:\n%2",
     "HTTP_BAD_SERVER_RESPONSE_RECEIVED", "bad response received when communicating with %1: %2",
     "HTTP_BAD_SERVER_RESPONSE_RECEIVED_DETAILS", "detailed information about bad response received from %1:\n%2",
+    "HTTP_CLIENT_MT_STARTED", "HttpClient has been started in multi-threaded mode running %1 threads",
     "HTTP_CLIENT_REQUEST_AUTHORIZED", "received HTTP request authorized for '%1'",
     "HTTP_CLIENT_REQUEST_BAD_AUTH_HEADER", "received HTTP request with malformed authentication header: %1",
     "HTTP_CLIENT_REQUEST_NOT_AUTHORIZED", "received HTTP request with not matching authentication header",
@@ -66,7 +68,7 @@ const char* values[] = {
     "HTTP_CONNECTION_STOP_FAILED", "stopping HTTP connection failed",
     "HTTP_DATA_RECEIVED", "received %1 bytes from %2",
     "HTTP_IDLE_CONNECTION_TIMEOUT_OCCURRED", "closing persistent connection with %1 as a result of a timeout",
-    "HTTP_PREMATURE_CONNECTION_TIMEOUT_OCCURRED", "premature connection timeout occurred, possibly caused by system clock change",
+    "HTTP_PREMATURE_CONNECTION_TIMEOUT_OCCURRED", "premature connection timeout occurred: in transaction ? %1, transid: %2, current_transid_: %3",
     "HTTP_REQUEST_RECEIVE_START", "start receiving request from %1 with timeout %2",
     "HTTP_SERVER_RESPONSE_RECEIVED", "received HTTP response from %1",
     "HTTP_SERVER_RESPONSE_RECEIVED_DETAILS", "detailed information about well-formed response received from %1:\n%2",
diff --git a/src/lib/http/http_messages.h b/src/lib/http/http_messages.h
index 9a4cc43107..5735df006b 100644
--- a/src/lib/http/http_messages.h
+++ b/src/lib/http/http_messages.h
@@ -13,6 +13,7 @@ extern const isc::log::MessageID HTTP_BAD_CLIENT_REQUEST_RECEIVED;
 extern const isc::log::MessageID HTTP_BAD_CLIENT_REQUEST_RECEIVED_DETAILS;
 extern const isc::log::MessageID HTTP_BAD_SERVER_RESPONSE_RECEIVED;
 extern const isc::log::MessageID HTTP_BAD_SERVER_RESPONSE_RECEIVED_DETAILS;
+extern const isc::log::MessageID HTTP_CLIENT_MT_STARTED;
 extern const isc::log::MessageID HTTP_CLIENT_REQUEST_AUTHORIZED;
 extern const isc::log::MessageID HTTP_CLIENT_REQUEST_BAD_AUTH_HEADER;
 extern const isc::log::MessageID HTTP_CLIENT_REQUEST_NOT_AUTHORIZED;
diff --git a/src/lib/http/tests/mt_client_unittests.cc b/src/lib/http/tests/mt_client_unittests.cc
index b3aec556c6..69295ddfb4 100644
--- a/src/lib/http/tests/mt_client_unittests.cc
+++ b/src/lib/http/tests/mt_client_unittests.cc
@@ -312,7 +312,8 @@ public:
 
         HttpResponseJsonPtr response_json = boost::make_shared<HttpResponseJson>();
 
-        ASSERT_NO_THROW(client_->asyncSendRequest(url, request, response_json,
+        ASSERT_NO_THROW(client_->asyncSendRequest(url, TlsContextPtr(),
+                                                  request, response_json,
             [this, request](const boost::system::error_code& ec,
                    const HttpResponsePtr& response,
                    const std::string&) {
@@ -375,9 +376,12 @@ public:
         factory_.reset(new TestHttpResponseCreatorFactory());
     
         // Need to create a Listener on
-        listener_.reset(new HttpListener(io_service_, IOAddress(SERVER_ADDRESS), SERVER_PORT,
-                                         factory_, HttpListener::RequestTimeout(10000),
+        listener_.reset(new HttpListener(io_service_, 
+                                         IOAddress(SERVER_ADDRESS), SERVER_PORT,
+                                         TlsContextPtr(), factory_,
+                                         HttpListener::RequestTimeout(10000),
                                          HttpListener::IdleTimeout(10000)));
+
         // Start the server.
         ASSERT_NO_THROW(listener_->start());
 
@@ -412,6 +416,8 @@ public:
             ASSERT_TRUE(sequence);
         }
 
+        ASSERT_NO_THROW(client_->stop());
+
         ASSERT_NO_THROW(listener_->stop());
     }