Merge topic 'libuv-raii'

1e9b7d3c server: Switched to a auto model for handles
f43b9219 tests: Added tests to verify UV RAII semantics/constructs
a3abb85c Add RAII handles for libuv handle types
90f8db26 tests: unconditionally enabled server tests
b56b51fc utility: Disabled copy ctors in thread classes

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !1453

4 files changed, 186 insertions, 6 deletions

diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt
index a61864f..9507880 100644
--- a/Tests/CMakeLists.txt
+++ b/Tests/CMakeLists.txt
@@ -149,9 +149,7 @@ if(BUILD_TESTING)
   if(NOT CMake_TEST_EXTERNAL_CMAKE)
     add_subdirectory(CMakeLib)
 
-    if(CMake_TEST_SERVER_MODE)
-      add_subdirectory(CMakeServerLib)
-    endif()
+    add_subdirectory(CMakeServerLib)
   endif()
   add_subdirectory(CMakeOnly)
   add_subdirectory(RunCMake)
diff --git a/Tests/CMakeServerLib/CMakeLists.txt b/Tests/CMakeServerLib/CMakeLists.txt
index f5351fd..f1ca2a4 100644
--- a/Tests/CMakeServerLib/CMakeLists.txt
+++ b/Tests/CMakeServerLib/CMakeLists.txt
@@ -6,12 +6,16 @@ include_directories(
 
 set(CMakeServerLib_TESTS
   testServerBuffering
+  testUVRAII
   )
 
 create_test_sourcelist(CMakeLib_TEST_SRCS CMakeServerLibTests.cxx ${CMakeServerLib_TESTS})
 add_executable(CMakeServerLibTests ${CMakeLib_TEST_SRCS})
 target_link_libraries(CMakeServerLibTests CMakeLib CMakeServerLib)
 
+SET_PROPERTY(TARGET CMakeServerLibTests PROPERTY C_CLANG_TIDY "")
+SET_PROPERTY(TARGET CMakeServerLibTests PROPERTY CXX_CLANG_TIDY "")
+
 foreach(test ${CMakeServerLib_TESTS})
   add_test(CMakeServerLib.${test} CMakeServerLibTests ${test} ${${test}_ARGS})
 endforeach()
diff --git a/Tests/CMakeServerLib/testServerBuffering.cpp b/Tests/CMakeServerLib/testServerBuffering.cpp
index 97be891..7330ead 100644
--- a/Tests/CMakeServerLib/testServerBuffering.cpp
+++ b/Tests/CMakeServerLib/testServerBuffering.cpp
@@ -1,7 +1,6 @@
 #include "cmConnection.h"
 #include "cmServerConnection.h"
 #include <iostream>
-#include <stddef.h>
 #include <string>
 #include <vector>
 
@@ -51,8 +50,8 @@ int testServerBuffering(int, char** const)
     std::unique_ptr<cmConnectionBufferStrategy>(new cmServerBufferStrategy);
   std::vector<std::string> response;
   std::string rawBuffer;
-  for (size_t i = 0; i < fullMessage.size(); i++) {
-    rawBuffer += fullMessage[i];
+  for (auto& messageChar : fullMessage) {
+    rawBuffer += messageChar;
     std::string packet = bufferingStrategy->BufferMessage(rawBuffer);
     do {
       if (!packet.empty() && packet != "\r\n") {
diff --git a/Tests/CMakeServerLib/testUVRAII.cxx b/Tests/CMakeServerLib/testUVRAII.cxx
new file mode 100644
index 0000000..7ecef39
--- /dev/null
+++ b/Tests/CMakeServerLib/testUVRAII.cxx
@@ -0,0 +1,179 @@
+#include "cmUVHandlePtr.h"
+
+#include <algorithm>
+#include <chrono>
+#include <iostream>
+#include <thread>
+
+#include "cm_uv.h"
+
+static void signal_reset_fn(uv_async_t* handle)
+{
+  auto ptr = static_cast<cm::uv_async_ptr*>(handle->data);
+  ptr->reset();
+}
+
+// A common pattern is to use an async signal to shutdown the server.
+static bool testAsyncShutdown()
+{
+  uv_loop_t Loop;
+  auto err = uv_loop_init(&Loop);
+  if (err != 0) {
+    std::cerr << "Could not init loop" << std::endl;
+    return false;
+  }
+
+  {
+    cm::uv_async_ptr signal;
+    signal.init(Loop, &signal_reset_fn, &signal);
+
+    std::thread([&] {
+      std::this_thread::sleep_for(std::chrono::seconds(2));
+      signal.send();
+    }).detach();
+
+    if (uv_run(&Loop, UV_RUN_DEFAULT) != 0) {
+      std::cerr << "Unclean exit state in testAsyncDtor" << std::endl;
+      return false;
+    }
+
+    if (signal.get()) {
+      std::cerr << "Loop exited with signal not being cleaned up" << std::endl;
+      return false;
+    }
+  }
+
+  uv_loop_close(&Loop);
+
+  return true;
+}
+
+static void signal_fn(uv_async_t*)
+{
+}
+
+// Async dtor is sort of a pain; since it locks a mutex we must be sure its
+// dtor always calls reset otherwise the mutex is deleted then locked.
+static bool testAsyncDtor()
+{
+  uv_loop_t Loop;
+  auto err = uv_loop_init(&Loop);
+  if (err != 0) {
+    std::cerr << "Could not init loop" << std::endl;
+    return false;
+  }
+
+  {
+    cm::uv_async_ptr signal;
+    signal.init(Loop, signal_fn);
+  }
+
+  if (uv_run(&Loop, UV_RUN_DEFAULT) != 0) {
+    std::cerr << "Unclean exit state in testAsyncDtor" << std::endl;
+    return false;
+  }
+
+  uv_loop_close(&Loop);
+
+  return true;
+}
+
+// Async needs a relatively stateful deleter; make sure that is properly
+// accounted for and doesn't try to hold on to invalid state when it is
+// moved
+static bool testAsyncMove()
+{
+  uv_loop_t Loop;
+  auto err = uv_loop_init(&Loop);
+  if (err != 0) {
+    std::cerr << "Could not init loop" << std::endl;
+    return false;
+  }
+
+  {
+    cm::uv_async_ptr signal;
+    {
+      cm::uv_async_ptr signalTmp;
+      signalTmp.init(Loop, signal_fn);
+      signal = std::move(signalTmp);
+    }
+  }
+
+  if (uv_run(&Loop, UV_RUN_DEFAULT) != 0) {
+    std::cerr << "Unclean exit state in testAsyncDtor" << std::endl;
+    return false;
+  }
+
+  uv_loop_close(&Loop);
+  return true;
+}
+
+// When a type is castable to another uv type (pipe -> stream) here,
+// and the deleter is convertible as well, we should allow moves from
+// one type to the other.
+static bool testCrossAssignment()
+{
+  uv_loop_t Loop;
+  auto err = uv_loop_init(&Loop);
+  if (err != 0) {
+    std::cerr << "Could not init loop" << std::endl;
+    return false;
+  }
+
+  {
+    cm::uv_pipe_ptr pipe;
+    pipe.init(Loop, 0);
+
+    cm::uv_stream_ptr stream = std::move(pipe);
+    if (pipe.get()) {
+      std::cerr << "Move should be sure to invalidate the previous ptr"
+                << std::endl;
+      return false;
+    }
+    cm::uv_handle_ptr handle = std::move(stream);
+    if (stream.get()) {
+      std::cerr << "Move should be sure to invalidate the previous ptr"
+                << std::endl;
+      return false;
+    }
+  }
+
+  if (uv_run(&Loop, UV_RUN_DEFAULT) != 0) {
+    std::cerr << "Unclean exit state in testCrossAssignment" << std::endl;
+    return false;
+  }
+
+  uv_loop_close(&Loop);
+  return true;
+}
+
+// This test can't fail at run time; but this makes sure we have all our move
+// ctors created correctly.
+static bool testAllMoves()
+{
+  using namespace cm;
+  struct allTypes
+  {
+    uv_stream_ptr _7;
+    uv_tty_ptr _9;
+    uv_pipe_ptr _12;
+    uv_async_ptr _13;
+    uv_signal_ptr _14;
+    uv_handle_ptr _15;
+  };
+
+  allTypes a;
+  allTypes b(std::move(a));
+  allTypes c = std::move(b);
+  return true;
+};
+
+int testUVRAII(int, char** const)
+{
+  if ((testAsyncShutdown() &&
+       testAsyncDtor() & testAsyncMove() & testCrossAssignment() &
+         testAllMoves()) == 0) {
+    return -1;
+  }
+  return 0;
+}