Bug fix #6: Keep connect operation cancelable even if it fails early …

…(during DNS lookup) in util/event_loop_posix.cpp
realm · May 23, 2016 · 9d54890 · 9d54890
1 parent 2e5fb6e
commit 9d54890
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 7 deletions.
diff --git a/src/realm/util/event_loop_posix.cpp b/src/realm/util/event_loop_posix.cpp
@@ -1,3 +1,5 @@
+#include <memory>
+
 #include <realm/util/network.hpp>
 #include <realm/util/event_loop.hpp>
 
@@ -60,7 +62,8 @@ class SocketImpl: public Socket {
     SocketImpl(EventLoopImpl& event_loop):
         m_event_loop(event_loop),
         m_socket(event_loop.get_service()), // Throws
-        m_input_stream(m_socket) // Throws
+        m_input_stream(m_socket), // Throws
+        m_shared_error_code(std::make_shared<std::error_code>()) // Throws
     {
     }
 
@@ -86,12 +89,20 @@ class SocketImpl: public Socket {
         std::error_code ec;
         resolver.resolve(query, m_endpoints, ec);
         if (ec) {
-            // Direct callbacks are not allowed
-            //
-            // FIXME: Use [handler=std::move(handler), ec] in C++14 to avoid a
-            // potentially expensive copy of user specified completion handler
-            // into the lambda
-            auto handler_2 = [=] {
+            // Direct callbacks are not allowed, so we need to postpone it using
+            // post(). Also, we need to make use of an error code object whose
+            // ownership is shared betweeen this socket object and the post
+            // handler, because connect completion handlers must be cancaleable
+            // up until the point int time where it starts to execute.
+            std::shared_ptr<std::error_code> sec = m_shared_error_code;
+            REALM_ASSERT(!*sec);
+            *sec = ec;
+            // FIXME: Use [handler=std::move(handler), sec=std::move(sec)] in
+            // C++14 to avoid a potentially expensive copy of user specified
+            // completion handler into the lambda
+            auto handler_2 = [handler, sec] {
+                std::error_code ec = *sec;
+                *sec = std::error_code();
                 handler(ec); // Throws
             };
             m_socket.service().post(std::move(handler_2)); // Throws
@@ -131,6 +142,7 @@ class SocketImpl: public Socket {
 
     void cancel() noexcept override final
     {
+        *m_shared_error_code = error::operation_aborted;
         if (m_connect_in_progress) {
             m_socket.close();
             m_connect_in_progress = false;
@@ -151,6 +163,7 @@ class SocketImpl: public Socket {
     network::buffered_input_stream m_input_stream;
     network::endpoint::list m_endpoints;
     bool m_connect_in_progress = false;
+    std::shared_ptr<std::error_code> m_shared_error_code;
 
     void try_next_endpoint(network::endpoint::list::iterator i, ConnectCompletionHandler handler)
     {

diff --git a/test/test_util_event_loop.cpp b/test/test_util_event_loop.cpp
@@ -330,6 +330,25 @@ TEST_TYPES(EventLoop_Connect_Failure, IMPLEMENTATIONS)
 }
 
 
+TEST_TYPES(EventLoop_Connect_EarlyFailureAndCancel, IMPLEMENTATIONS)
+{
+    // Check that an early failure (during DNS lookup) does not make the
+    // operation uncancaleable.
+
+    std::unique_ptr<EventLoop> event_loop = MakeEventLoop<TEST_TYPE>()();
+    std::unique_ptr<Socket> socket = event_loop->make_socket();
+
+    std::string bad_host_name = "G6iHvNo9dEVPfL3EcWHWSChQU3GD8cjEGuy92eeTeaVWGPt71kJa5MykiYHGlg3M";
+
+    std::error_code ec;
+    socket->async_connect(std::move(bad_host_name), 0, SocketSecurity::None,
+                          [&](std::error_code ec_2) { ec = ec_2; });
+    socket->cancel();
+    event_loop->run();
+    CHECK_EQUAL(error::operation_aborted, ec);
+}
+
+
 TEST_TYPES(EventLoop_DeadlineTimer, IMPLEMENTATIONS)
 {
     std::unique_ptr<EventLoop> event_loop = MakeEventLoop<TEST_TYPE>()();