Browse Source

[master] Merge branch 'trac5217'

Marcin Siodelski 8 years ago
parent
commit
4bcb45f0c8

+ 12 - 3
src/lib/asiolink/tests/tcp_acceptor_unittest.cc

@@ -93,9 +93,18 @@ public:
     /// @param ec Error code.
     void connectHandler(const boost::system::error_code& ec) {
         if (ec) {
-            ADD_FAILURE() << "error occurred while connecting: "
-                          << ec.message();
-            io_service_.stop();
+            // One would expect that async_connect wouldn't return EINPROGRESS
+            // error code, but simply wait for the connection to get
+            // established before the handler is invoked. It turns out, however,
+            // that on some OSes the connect handler may receive this error code
+            // which doesn't neccessarily indicate a problem. Making an attempt
+            // to write and read from this socket will typically succeed. So,
+            // we ignore this error.
+            if (ec.value() != boost::asio::error::in_progress) {
+                ADD_FAILURE() << "error occurred while connecting: "
+                              << ec.message();
+                io_service_.stop();
+            }
         }
     }
 

+ 14 - 5
src/lib/asiolink/tests/tcp_socket_unittest.cc

@@ -22,14 +22,15 @@
 #include <boost/shared_ptr.hpp>
 #include <gtest/gtest.h>
 
-#include <string>
+#include <algorithm>
 #include <arpa/inet.h>
+#include <cstddef>
+#include <cstdlib>
+#include <errno.h>
 #include <netinet/in.h>
 #include <sys/types.h>
 #include <sys/socket.h>
-#include <algorithm>
-#include <cstdlib>
-#include <cstddef>
+#include <string>
 #include <vector>
 
 using namespace boost::asio;
@@ -355,7 +356,15 @@ TEST(TCPSocket, sequenceTest) {
     EXPECT_EQ(0, server_cb.getCode());
 
     EXPECT_EQ(TCPCallback::OPEN, client_cb.called());
-    EXPECT_EQ(0, client_cb.getCode());
+
+    // On some operating system the async_connect may return EINPROGRESS.
+    // This doesn't neccessarily indicate an error. In most cases trying
+    // to asynchrouonsly write and read from the socket would work just
+    // fine.
+    if ((client_cb.getCode()) != 0 && (client_cb.getCode() != EINPROGRESS)) {
+        ADD_FAILURE() << "expected error code of 0 or " << EINPROGRESS
+            << " as a result of async_connect, got " << client_cb.getCode();
+    }
 
     // Step 2.  Get the client to write to the server asynchronously.  The
     // server will loop reading the data synchronously.

+ 47 - 6
src/lib/http/connection.cc

@@ -140,16 +140,39 @@ HttpConnection::acceptorCallback(const boost::system::error_code& ec) {
 }
 
 void
-HttpConnection::socketReadCallback(boost::system::error_code, size_t length) {
+HttpConnection::socketReadCallback(boost::system::error_code ec, size_t length) {
+    if (ec) {
+        // IO service has been stopped and the connection is probably
+        // going to be shutting down.
+        if (ec.value() == boost::asio::error::operation_aborted) {
+            return;
+
+        // EWOULDBLOCK and EAGAIN are special cases. Everything else is
+        // treated as fatal error.
+        } else if ((ec.value() != boost::asio::error::try_again) &&
+                   (ec.value() != boost::asio::error::would_block)) {
+            stopThisConnection();
+
+        // We got EWOULDBLOCK or EAGAIN which indicate that we may be able to
+        // read something from the socket on the next attempt. Just make sure
+        // we don't try to read anything now in case there is any garbage
+        // passed in length.
+        } else {
+            length = 0;
+        }
+    }
+
     if (length != 0) {
         LOG_DEBUG(http_logger, isc::log::DBGLVL_TRACE_DETAIL_DATA,
                   HTTP_DATA_RECEIVED)
             .arg(length)
             .arg(getRemoteEndpointAddressAsText());
+
+        std::string s(&buf_[0], buf_[0] + length);
+        parser_->postBuffer(static_cast<void*>(buf_.data()), length);
+        parser_->poll();
     }
-    std::string s(&buf_[0], buf_[0] + length);
-    parser_->postBuffer(static_cast<void*>(buf_.data()), length);
-    parser_->poll();
+
     if (parser_->needData()) {
         doRead();
 
@@ -172,8 +195,26 @@ HttpConnection::socketReadCallback(boost::system::error_code, size_t length) {
 }
 
 void
-HttpConnection::socketWriteCallback(boost::system::error_code,
-                                    size_t length) {
+HttpConnection::socketWriteCallback(boost::system::error_code ec, size_t length) {
+    if (ec) {
+        // IO service has been stopped and the connection is probably
+        // going to be shutting down.
+        if (ec.value() == boost::asio::error::operation_aborted) {
+            return;
+
+        // EWOULDBLOCK and EAGAIN are special cases. Everything else is
+        // treated as fatal error.
+        } else if ((ec.value() != boost::asio::error::try_again) &&
+                   (ec.value() != boost::asio::error::would_block)) {
+            stopThisConnection();
+
+        // We got EWOULDBLOCK or EAGAIN which indicate that we may be able to
+        // read something from the socket on the next attempt.
+        } else {
+            doWrite();
+        }
+    }
+
     if (length <= output_buf_.size()) {
         output_buf_.erase(0, length);
         doWrite();

+ 2 - 1
src/lib/http/request_parser.cc

@@ -78,7 +78,8 @@ HttpRequestParser::poll() {
 
 bool
 HttpRequestParser::needData() const {
-    return (getNextEvent() == NEED_MORE_DATA_EVT);
+    return ((getNextEvent() == NEED_MORE_DATA_EVT) ||
+            (getNextEvent() == START_EVT));
 }
 
 bool

+ 14 - 4
src/lib/http/tests/listener_unittests.cc

@@ -133,11 +133,21 @@ public:
         tcp::endpoint endpoint(address::from_string(SERVER_ADDRESS),
                                SERVER_PORT);
         socket_.async_connect(endpoint,
-                              [this, request](const boost::system::error_code& ec) {
+        [this, request](const boost::system::error_code& ec) {
             if (ec) {
-                ADD_FAILURE() << "error occurred while connecting: "
-                              << ec.message();
-                io_service_.stop();
+                // One would expect that async_connect wouldn't return
+                // EINPROGRESS error code, but simply wait for the connection
+                // to get established before the handler is invoked. It turns out,
+                // however, that on some OSes the connect handler may receive this
+                // error code which doesn't neccessarily indicate a problem.
+                // Making an attempt to write and read from this socket will
+                // typically succeed. So, we ignore this error.
+                if (ec.value() != boost::asio::error::in_progress) {
+                    ADD_FAILURE() << "error occurred while connecting: "
+                                  << ec.message();
+                    io_service_.stop();
+                    return;
+                }
             }
             sendRequest(request);
         });