From b56b51fcafabc678e90474871cd282450949152a Mon Sep 17 00:00:00 2001
From: Justin Berger <j.david.berger@gmail.com>
Date: Fri, 17 Nov 2017 13:43:51 -0700
Subject: utility: Disabled copy ctors in thread classes
---
Source/cm_thread.hxx | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Source/cm_thread.hxx b/Source/cm_thread.hxx
index b8c25c7..ec5fe1d 100644
--- a/Source/cm_thread.hxx
+++ b/Source/cm_thread.hxx
@@ -9,6 +9,7 @@
namespace cm {
class mutex
{
+ CM_DISABLE_COPY(mutex)
uv_mutex_t _M_;
public:
@@ -24,6 +25,7 @@ template <typename T>
class lock_guard
{
T& _mutex;
+ CM_DISABLE_COPY(lock_guard)
public:
lock_guard(T& m)
@@ -37,6 +39,7 @@ public:
class shared_mutex
{
uv_rwlock_t _M_;
+ CM_DISABLE_COPY(shared_mutex)
public:
shared_mutex() { uv_rwlock_init(&_M_); }
@@ -55,6 +58,7 @@ template <typename T>
class shared_lock
{
T& _mutex;
+ CM_DISABLE_COPY(shared_lock)
public:
shared_lock(T& m)
@@ -68,6 +72,8 @@ public:
template <typename T>
class unique_lock : public lock_guard<T>
{
+ CM_DISABLE_COPY(unique_lock)
+
public:
unique_lock(T& m)
: lock_guard<T>(m)
--
cgit v0.12
From 90f8db269fcc6978badd93b5f985dee2e01feab3 Mon Sep 17 00:00:00 2001
From: Justin Berger <j.david.berger@gmail.com>
Date: Thu, 9 Nov 2017 20:52:19 -0700
Subject: tests: unconditionally enabled server tests
---
Tests/CMakeLists.txt | 4 +---
Tests/CMakeServerLib/testServerBuffering.cpp | 5 ++---
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt
index 4a7b8c9..18fed75 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/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") {
--
cgit v0.12
From a3abb85c6ff5aebe2220ae612512a434b491eedd Mon Sep 17 00:00:00 2001
From: Justin Berger <j.david.berger@gmail.com>
Date: Thu, 27 Jul 2017 07:11:07 -0600
Subject: Add RAII handles for libuv handle types
The `uv_*_t` handle types are closed by `uv_close`, but the semantics
are tricky. Calling `uv_close` may not close immediately. Instead it
hands ownership to the uv loop to which the handle is currently
attached. When the loop decides to close it, a callback is used to
allow the `uv_close` caller to free resources.
Provide an abstraction layer as `cm::uv_*_ptr` types corresponding to
the `uv_*_t` handle types. Each pointer is either empty (`nullptr`)
or has an initialized handle attached to a loop. Use move semantics
to ensure a single owner of the handle so that clients can predict
when the handle is destroyed.
---
Source/CMakeLists.txt | 1 +
Source/cmUVHandlePtr.cxx | 198 ++++++++++++++++++++++++++++++++++++++++++++++
Source/cmUVHandlePtr.h | 200 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 399 insertions(+)
create mode 100644 Source/cmUVHandlePtr.cxx
create mode 100644 Source/cmUVHandlePtr.h
diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt
index 33ab093..88c63e1 100644
--- a/Source/CMakeLists.txt
+++ b/Source/CMakeLists.txt
@@ -1029,6 +1029,7 @@ list(APPEND _tools cmake)
target_link_libraries(cmake CMakeLib)
add_library(CMakeServerLib
+ cmUVHandlePtr.h cmUVHandlePtr.cxx
cmConnection.h cmConnection.cxx
cmFileMonitor.cxx cmFileMonitor.h
cmPipeConnection.cxx cmPipeConnection.h
diff --git a/Source/cmUVHandlePtr.cxx b/Source/cmUVHandlePtr.cxx
new file mode 100644
index 0000000..214c7b2
--- /dev/null
+++ b/Source/cmUVHandlePtr.cxx
@@ -0,0 +1,198 @@
+/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying
+ file Copyright.txt or https://cmake.org/licensing for details. */
+#define cmUVHandlePtr_cxx
+#include "cmUVHandlePtr.h"
+
+#include <assert.h>
+#include <stdlib.h>
+
+#include "cm_thread.hxx"
+#include "cm_uv.h"
+
+namespace cm {
+
+static void close_delete(uv_handle_t* h)
+{
+ free(h);
+}
+
+template <typename T>
+static void default_delete(T* type_handle)
+{
+ auto handle = reinterpret_cast<uv_handle_t*>(type_handle);
+ if (handle) {
+ assert(!uv_is_closing(handle));
+ if (!uv_is_closing(handle)) {
+ uv_close(handle, &close_delete);
+ }
+ }
+}
+
+/**
+ * Encapsulates delete logic for a given handle type T
+ */
+template <typename T>
+struct uv_handle_deleter
+{
+ void operator()(T* type_handle) const { default_delete(type_handle); }
+};
+
+template <typename T>
+void uv_handle_ptr_base_<T>::allocate(void* data)
+{
+ reset();
+
+ /*
+ We use calloc since we know all these types are c structs
+ and we just want to 0 init them. New would do the same thing;
+ but casting from uv_handle_t to certain other types -- namely
+ uv_timer_t -- triggers a cast_align warning on certain systems.
+ */
+ handle.reset(static_cast<T*>(calloc(1, sizeof(T))), uv_handle_deleter<T>());
+ handle->data = data;
+}
+
+template <typename T>
+void uv_handle_ptr_base_<T>::reset()
+{
+ handle.reset();
+}
+
+template <typename T>
+uv_handle_ptr_base_<T>::operator uv_handle_t*()
+{
+ return reinterpret_cast<uv_handle_t*>(handle.get());
+}
+
+template <typename T>
+T* uv_handle_ptr_base_<T>::operator->() const noexcept
+{
+ return handle.get();
+}
+
+template <typename T>
+T* uv_handle_ptr_base_<T>::get() const
+{
+ return handle.get();
+}
+
+template <typename T>
+uv_handle_ptr_<T>::operator T*() const
+{
+ return this->handle.get();
+}
+
+template <>
+struct uv_handle_deleter<uv_async_t>
+{
+ /***
+ * Wile uv_async_send is itself thread-safe, there are
+ * no strong guarantees that close hasn't already been
+ * called on the handle; and that it might be deleted
+ * as the send call goes through. This mutex guards
+ * against that.
+ *
+ * The shared_ptr here is to allow for copy construction
+ * which is mandated by the standard for Deleter on
+ * shared_ptrs.
+ */
+ std::shared_ptr<cm::mutex> handleMutex;
+
+ uv_handle_deleter()
+ : handleMutex(std::make_shared<cm::mutex>())
+ {
+ }
+
+ void operator()(uv_async_t* handle)
+ {
+ cm::lock_guard<cm::mutex> lock(*handleMutex);
+ default_delete(handle);
+ }
+};
+
+void uv_async_ptr::send()
+{
+ auto deleter = std::get_deleter<uv_handle_deleter<uv_async_t>>(this->handle);
+ assert(deleter);
+
+ cm::lock_guard<cm::mutex> lock(*deleter->handleMutex);
+ if (this->handle) {
+ uv_async_send(*this);
+ }
+}
+
+int uv_async_ptr::init(uv_loop_t& loop, uv_async_cb async_cb, void* data)
+{
+ allocate(data);
+ return uv_async_init(&loop, handle.get(), async_cb);
+}
+
+template <>
+struct uv_handle_deleter<uv_signal_t>
+{
+ void operator()(uv_signal_t* handle) const
+ {
+ if (handle) {
+ uv_signal_stop(handle);
+ default_delete(handle);
+ }
+ }
+};
+
+int uv_signal_ptr::init(uv_loop_t& loop, void* data)
+{
+ allocate(data);
+ return uv_signal_init(&loop, handle.get());
+}
+
+int uv_signal_ptr::start(uv_signal_cb cb, int signum)
+{
+ assert(handle);
+ return uv_signal_start(*this, cb, signum);
+}
+
+void uv_signal_ptr::stop()
+{
+ if (handle) {
+ uv_signal_stop(*this);
+ }
+}
+
+int uv_pipe_ptr::init(uv_loop_t& loop, int ipc, void* data)
+{
+ allocate(data);
+ return uv_pipe_init(&loop, *this, ipc);
+}
+
+uv_pipe_ptr::operator uv_stream_t*() const
+{
+ return reinterpret_cast<uv_stream_t*>(handle.get());
+}
+
+uv_tty_ptr::operator uv_stream_t*() const
+{
+ return reinterpret_cast<uv_stream_t*>(handle.get());
+}
+
+int uv_tty_ptr::init(uv_loop_t& loop, int fd, int readable, void* data)
+{
+ allocate(data);
+ return uv_tty_init(&loop, *this, fd, readable);
+}
+
+template class uv_handle_ptr_base_<uv_handle_t>;
+
+#define UV_HANDLE_PTR_INSTANTIATE_EXPLICIT(NAME) \
+ template class uv_handle_ptr_base_<uv_##NAME##_t>; \
+ template class uv_handle_ptr_<uv_##NAME##_t>;
+
+UV_HANDLE_PTR_INSTANTIATE_EXPLICIT(async)
+
+UV_HANDLE_PTR_INSTANTIATE_EXPLICIT(signal)
+
+UV_HANDLE_PTR_INSTANTIATE_EXPLICIT(pipe)
+
+UV_HANDLE_PTR_INSTANTIATE_EXPLICIT(stream)
+
+UV_HANDLE_PTR_INSTANTIATE_EXPLICIT(tty)
+}
diff --git a/Source/cmUVHandlePtr.h b/Source/cmUVHandlePtr.h
new file mode 100644
index 0000000..a251c5f
--- /dev/null
+++ b/Source/cmUVHandlePtr.h
@@ -0,0 +1,200 @@
+/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying
+ file Copyright.txt or https://cmake.org/licensing for details. */
+#pragma once
+#include "cmConfigure.h" // IWYU pragma: keep
+
+#include <algorithm>
+#include <cstddef>
+#include <memory>
+#include <type_traits>
+
+#include "cm_uv.h"
+
+#define CM_PERFECT_FWD_CTOR(Class, FwdTo) \
+ template <typename... Args> \
+ Class(Args&&... args) \
+ : FwdTo(std::forward<Args>(args)...) \
+ { \
+ }
+
+namespace cm {
+
+/***
+* RAII class to simplify and insure the safe usage of uv_*_t types. This
+* includes making sure resources are properly freed and contains casting
+* operators which allow for passing into relevant uv_* functions.
+*
+*@tparam T actual uv_*_t type represented.
+*/
+template <typename T>
+class uv_handle_ptr_base_
+{
+protected:
+ template <typename _T>
+ friend class uv_handle_ptr_base_;
+
+ /**
+ * This must be a pointer type since the handle can outlive this class.
+ * When uv_close is eventually called on the handle, the memory the
+ * handle inhabits must be valid until the close callback is called
+ * which can be later on in the loop.
+ */
+ std::shared_ptr<T> handle;
+
+ /**
+ * Allocate memory for the type and optionally set it's 'data' pointer.
+ * Protected since this should only be called for an appropriate 'init'
+ * call.
+ *
+ * @param data data pointer to set
+ */
+ void allocate(void* data = nullptr);
+
+public:
+ CM_DISABLE_COPY(uv_handle_ptr_base_)
+ uv_handle_ptr_base_(uv_handle_ptr_base_&&) noexcept;
+ uv_handle_ptr_base_& operator=(uv_handle_ptr_base_&&) noexcept;
+
+ /**
+ * This move constructor allows us to move out of a more specialized
+ * uv type into a less specialized one. The only constraint is that
+ * the right hand side is castable to T.
+ *
+ * This allows you to return uv_handle_ptr or uv_stream_ptr from a function
+ * that initializes something like uv_pipe_ptr or uv_tcp_ptr and interact
+ * and clean up after it without caring about the exact type.
+ */
+ template <typename S, typename = typename std::enable_if<
+ std::is_rvalue_reference<S&&>::value>::type>
+ uv_handle_ptr_base_(S&& rhs)
+ {
+ // This will force a compiler error if rhs doesn't have a casting
+ // operator to get T*
+ this->handle = std::shared_ptr<T>(rhs.handle, rhs);
+ rhs.handle.reset();
+ }
+
+ // Dtor and ctor need to be inline defined like this for default ctors and
+ // dtors to work.
+ uv_handle_ptr_base_() {}
+ uv_handle_ptr_base_(std::nullptr_t) {}
+ ~uv_handle_ptr_base_() { reset(); }
+
+ /**
+ * Properly close the handle if needed and sets the inner handle to nullptr
+ */
+ void reset();
+
+ /**
+ * Allow less verbose calling of uv_handle_* functions
+ * @return reinterpreted handle
+ */
+ operator uv_handle_t*();
+
+ T* get() const;
+ T* operator->() const noexcept;
+};
+
+template <typename T>
+inline uv_handle_ptr_base_<T>::uv_handle_ptr_base_(
+ uv_handle_ptr_base_<T>&&) noexcept = default;
+template <typename T>
+inline uv_handle_ptr_base_<T>& uv_handle_ptr_base_<T>::operator=(
+ uv_handle_ptr_base_<T>&&) noexcept = default;
+
+/**
+ * While uv_handle_ptr_base_ only exposes uv_handle_t*, this exposes uv_T_t*
+ * too. It is broken out like this so we can reuse most of the code for the
+ * uv_handle_ptr class.
+ */
+template <typename T>
+class uv_handle_ptr_ : public uv_handle_ptr_base_<T>
+{
+ template <typename _T>
+ friend class uv_handle_ptr_;
+
+public:
+ CM_PERFECT_FWD_CTOR(uv_handle_ptr_, uv_handle_ptr_base_<T>);
+
+ /***
+ * Allow less verbose calling of uv_<T> functions
+ * @return reinterpreted handle
+ */
+ operator T*() const;
+};
+
+/***
+ * This specialization is required to avoid duplicate 'operator uv_handle_t*()'
+ * declarations
+ */
+template <>
+class uv_handle_ptr_<uv_handle_t> : public uv_handle_ptr_base_<uv_handle_t>
+{
+public:
+ CM_PERFECT_FWD_CTOR(uv_handle_ptr_, uv_handle_ptr_base_<uv_handle_t>);
+};
+
+class uv_async_ptr : public uv_handle_ptr_<uv_async_t>
+{
+public:
+ CM_PERFECT_FWD_CTOR(uv_async_ptr, uv_handle_ptr_<uv_async_t>);
+
+ int init(uv_loop_t& loop, uv_async_cb async_cb, void* data = nullptr);
+
+ void send();
+};
+
+struct uv_signal_ptr : public uv_handle_ptr_<uv_signal_t>
+{
+ CM_PERFECT_FWD_CTOR(uv_signal_ptr, uv_handle_ptr_<uv_signal_t>);
+
+ int init(uv_loop_t& loop, void* data = nullptr);
+
+ int start(uv_signal_cb cb, int signum);
+
+ void stop();
+};
+
+struct uv_pipe_ptr : public uv_handle_ptr_<uv_pipe_t>
+{
+ CM_PERFECT_FWD_CTOR(uv_pipe_ptr, uv_handle_ptr_<uv_pipe_t>);
+
+ operator uv_stream_t*() const;
+
+ int init(uv_loop_t& loop, int ipc, void* data = nullptr);
+};
+
+struct uv_tty_ptr : public uv_handle_ptr_<uv_tty_t>
+{
+ CM_PERFECT_FWD_CTOR(uv_tty_ptr, uv_handle_ptr_<uv_tty_t>);
+
+ operator uv_stream_t*() const;
+
+ int init(uv_loop_t& loop, int fd, int readable, void* data = nullptr);
+};
+
+typedef uv_handle_ptr_<uv_stream_t> uv_stream_ptr;
+typedef uv_handle_ptr_<uv_handle_t> uv_handle_ptr;
+
+#ifndef cmUVHandlePtr_cxx
+
+extern template class uv_handle_ptr_base_<uv_handle_t>;
+
+#define UV_HANDLE_PTR_INSTANTIATE_EXTERN(NAME) \
+ extern template class uv_handle_ptr_base_<uv_##NAME##_t>; \
+ extern template class uv_handle_ptr_<uv_##NAME##_t>;
+
+UV_HANDLE_PTR_INSTANTIATE_EXTERN(async)
+
+UV_HANDLE_PTR_INSTANTIATE_EXTERN(signal)
+
+UV_HANDLE_PTR_INSTANTIATE_EXTERN(pipe)
+
+UV_HANDLE_PTR_INSTANTIATE_EXTERN(stream)
+
+UV_HANDLE_PTR_INSTANTIATE_EXTERN(tty)
+
+#undef UV_HANDLE_PTR_INSTANTIATE_EXTERN
+
+#endif
+}
--
cgit v0.12
From f43b9219c738944fea33043b1985696c862c84ac Mon Sep 17 00:00:00 2001
From: Justin Berger <j.david.berger@gmail.com>
Date: Thu, 9 Nov 2017 20:52:32 -0700
Subject: tests: Added tests to verify UV RAII semantics/constructs
---
Tests/CMakeServerLib/CMakeLists.txt | 4 +
Tests/CMakeServerLib/testUVRAII.cxx | 179 ++++++++++++++++++++++++++++++++++++
2 files changed, 183 insertions(+)
create mode 100644 Tests/CMakeServerLib/testUVRAII.cxx
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/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;
+}
--
cgit v0.12
From 1e9b7d3ceb882d3feb59324b6e55d40cc795576b Mon Sep 17 00:00:00 2001
From: Justin Berger <j.david.berger@gmail.com>
Date: Sun, 23 Jul 2017 12:31:13 -0600
Subject: server: Switched to a auto model for handles
---
Source/cmConnection.cxx | 19 +++++-------
Source/cmConnection.h | 5 +--
Source/cmPipeConnection.cxx | 51 ++++++++++++-------------------
Source/cmPipeConnection.h | 4 +--
Source/cmServer.cxx | 67 ++++++++++++++++++++--------------------
Source/cmServer.h | 10 +++---
Source/cmServerConnection.cxx | 71 +++++++++++++------------------------------
Source/cmServerConnection.h | 6 ++--
8 files changed, 96 insertions(+), 137 deletions(-)
diff --git a/Source/cmConnection.cxx b/Source/cmConnection.cxx
index 28ba12c..50e1936 100644
--- a/Source/cmConnection.cxx
+++ b/Source/cmConnection.cxx
@@ -26,7 +26,7 @@ void cmEventBasedConnection::on_alloc_buffer(uv_handle_t* handle,
void cmEventBasedConnection::on_read(uv_stream_t* stream, ssize_t nread,
const uv_buf_t* buf)
{
- auto conn = reinterpret_cast<cmEventBasedConnection*>(stream->data);
+ auto conn = static_cast<cmEventBasedConnection*>(stream->data);
if (conn) {
if (nread >= 0) {
conn->ReadData(std::string(buf->base, buf->base + nread));
@@ -55,7 +55,7 @@ void cmEventBasedConnection::on_write(uv_write_t* req, int status)
void cmEventBasedConnection::on_new_connection(uv_stream_t* stream, int status)
{
(void)(status);
- auto conn = reinterpret_cast<cmEventBasedConnection*>(stream->data);
+ auto conn = static_cast<cmEventBasedConnection*>(stream->data);
if (conn) {
conn->Connect(stream);
@@ -76,7 +76,7 @@ void cmEventBasedConnection::WriteData(const std::string& _data)
#endif
auto data = _data;
- assert(this->WriteStream);
+ assert(this->WriteStream.get());
if (BufferStrategy) {
data = BufferStrategy->BufferOutMessage(data);
}
@@ -87,8 +87,7 @@ void cmEventBasedConnection::WriteData(const std::string& _data)
req->req.data = this;
req->buf = uv_buf_init(new char[ds], static_cast<unsigned int>(ds));
memcpy(req->buf.base, data.c_str(), ds);
- uv_write(reinterpret_cast<uv_write_t*>(req),
- static_cast<uv_stream_t*>(this->WriteStream), &req->buf, 1,
+ uv_write(reinterpret_cast<uv_write_t*>(req), this->WriteStream, &req->buf, 1,
on_write);
}
@@ -156,13 +155,11 @@ bool cmConnection::OnServeStart(std::string* errString)
bool cmEventBasedConnection::OnConnectionShuttingDown()
{
- if (this->WriteStream) {
+ if (this->WriteStream.get()) {
this->WriteStream->data = nullptr;
}
- if (this->ReadStream) {
- this->ReadStream->data = nullptr;
- }
- this->ReadStream = nullptr;
- this->WriteStream = nullptr;
+
+ WriteStream.reset();
+
return true;
}
diff --git a/Source/cmConnection.h b/Source/cmConnection.h
index ddb7744..ce2d2dc 100644
--- a/Source/cmConnection.h
+++ b/Source/cmConnection.h
@@ -5,6 +5,7 @@
#include "cmConfigure.h" // IWYU pragma: keep
+#include "cmUVHandlePtr.h"
#include "cm_uv.h"
#include <cstddef>
@@ -107,8 +108,6 @@ public:
bool OnConnectionShuttingDown() override;
virtual void OnDisconnect(int errorCode);
- uv_stream_t* ReadStream = nullptr;
- uv_stream_t* WriteStream = nullptr;
static void on_close(uv_handle_t* handle);
@@ -119,6 +118,8 @@ public:
}
protected:
+ cm::uv_stream_ptr WriteStream;
+
std::string RawReadBuffer;
std::unique_ptr<cmConnectionBufferStrategy> BufferStrategy;
diff --git a/Source/cmPipeConnection.cxx b/Source/cmPipeConnection.cxx
index 9e565f6..3dab2f0 100644
--- a/Source/cmPipeConnection.cxx
+++ b/Source/cmPipeConnection.cxx
@@ -2,6 +2,8 @@
file Copyright.txt or https://cmake.org/licensing for details. */
#include "cmPipeConnection.h"
+#include <algorithm>
+
#include "cmServer.h"
cmPipeConnection::cmPipeConnection(const std::string& name,
@@ -13,39 +15,33 @@ cmPipeConnection::cmPipeConnection(const std::string& name,
void cmPipeConnection::Connect(uv_stream_t* server)
{
- if (this->ClientPipe) {
+ if (this->WriteStream.get()) {
// Accept and close all pipes but the first:
- uv_pipe_t* rejectPipe = new uv_pipe_t();
+ cm::uv_pipe_ptr rejectPipe;
+
+ rejectPipe.init(*this->Server->GetLoop(), 0);
+ uv_accept(server, rejectPipe);
- uv_pipe_init(this->Server->GetLoop(), rejectPipe, 0);
- uv_accept(server, reinterpret_cast<uv_stream_t*>(rejectPipe));
- uv_close(reinterpret_cast<uv_handle_t*>(rejectPipe),
- &on_close_delete<uv_pipe_t>);
return;
}
- this->ClientPipe = new uv_pipe_t();
- uv_pipe_init(this->Server->GetLoop(), this->ClientPipe, 0);
- this->ClientPipe->data = static_cast<cmEventBasedConnection*>(this);
- auto client = reinterpret_cast<uv_stream_t*>(this->ClientPipe);
- if (uv_accept(server, client) != 0) {
- uv_close(reinterpret_cast<uv_handle_t*>(client),
- &on_close_delete<uv_pipe_t>);
- this->ClientPipe = nullptr;
+ cm::uv_pipe_ptr ClientPipe;
+ ClientPipe.init(*this->Server->GetLoop(), 0,
+ static_cast<cmEventBasedConnection*>(this));
+
+ if (uv_accept(server, ClientPipe) != 0) {
return;
}
- this->ReadStream = client;
- this->WriteStream = client;
- uv_read_start(this->ReadStream, on_alloc_buffer, on_read);
+ uv_read_start(ClientPipe, on_alloc_buffer, on_read);
+ WriteStream = std::move(ClientPipe);
Server->OnConnected(this);
}
bool cmPipeConnection::OnServeStart(std::string* errorMessage)
{
- this->ServerPipe = new uv_pipe_t();
- uv_pipe_init(this->Server->GetLoop(), this->ServerPipe, 0);
- this->ServerPipe->data = static_cast<cmEventBasedConnection*>(this);
+ this->ServerPipe.init(*this->Server->GetLoop(), 0,
+ static_cast<cmEventBasedConnection*>(this));
int r;
if ((r = uv_pipe_bind(this->ServerPipe, this->PipeName.c_str())) != 0) {
@@ -53,8 +49,8 @@ bool cmPipeConnection::OnServeStart(std::string* errorMessage)
": " + uv_err_name(r);
return false;
}
- auto serverStream = reinterpret_cast<uv_stream_t*>(this->ServerPipe);
- if ((r = uv_listen(serverStream, 1, on_new_connection)) != 0) {
+
+ if ((r = uv_listen(this->ServerPipe, 1, on_new_connection)) != 0) {
*errorMessage = std::string("Internal Error listening on ") +
this->PipeName + ": " + uv_err_name(r);
return false;
@@ -65,18 +61,11 @@ bool cmPipeConnection::OnServeStart(std::string* errorMessage)
bool cmPipeConnection::OnConnectionShuttingDown()
{
- if (this->ClientPipe) {
- uv_close(reinterpret_cast<uv_handle_t*>(this->ClientPipe),
- &on_close_delete<uv_pipe_t>);
+ if (this->WriteStream.get()) {
this->WriteStream->data = nullptr;
}
- uv_close(reinterpret_cast<uv_handle_t*>(this->ServerPipe),
- &on_close_delete<uv_pipe_t>);
- this->ClientPipe = nullptr;
- this->ServerPipe = nullptr;
- this->WriteStream = nullptr;
- this->ReadStream = nullptr;
+ this->ServerPipe.reset();
return cmEventBasedConnection::OnConnectionShuttingDown();
}
diff --git a/Source/cmPipeConnection.h b/Source/cmPipeConnection.h
index 7b89842..49f9fdf 100644
--- a/Source/cmPipeConnection.h
+++ b/Source/cmPipeConnection.h
@@ -4,6 +4,7 @@
#include "cmConfigure.h" // IWYU pragma: keep
+#include "cmUVHandlePtr.h"
#include <string>
#include "cmConnection.h"
@@ -23,6 +24,5 @@ public:
private:
const std::string PipeName;
- uv_pipe_t* ServerPipe = nullptr;
- uv_pipe_t* ClientPipe = nullptr;
+ cm::uv_pipe_ptr ServerPipe;
};
diff --git a/Source/cmServer.cxx b/Source/cmServer.cxx
index 9af4c0a..ac42fde 100644
--- a/Source/cmServer.cxx
+++ b/Source/cmServer.cxx
@@ -22,13 +22,14 @@
void on_signal(uv_signal_t* signal, int signum)
{
- auto conn = reinterpret_cast<cmServerBase*>(signal->data);
+ auto conn = static_cast<cmServerBase*>(signal->data);
conn->OnSignal(signum);
}
static void on_walk_to_shutdown(uv_handle_t* handle, void* arg)
{
(void)arg;
+ assert(uv_is_closing(handle));
if (!uv_is_closing(handle)) {
uv_close(handle, &cmEventBasedConnection::on_close);
}
@@ -58,6 +59,8 @@ cmServer::cmServer(cmConnection* conn, bool supportExperimental)
cmServer::~cmServer()
{
+ Close();
+
for (cmServerProtocol* p : this->SupportedProtocols) {
delete p;
}
@@ -409,7 +412,7 @@ void cmServer::StartShutDown()
static void __start_thread(void* arg)
{
- auto server = reinterpret_cast<cmServerBase*>(arg);
+ auto server = static_cast<cmServerBase*>(arg);
std::string error;
bool success = server->Serve(&error);
if (!success || error.empty() == false) {
@@ -417,22 +420,19 @@ static void __start_thread(void* arg)
}
}
-static void __shutdownThread(uv_async_t* arg)
-{
- auto server = reinterpret_cast<cmServerBase*>(arg->data);
- on_walk_to_shutdown(reinterpret_cast<uv_handle_t*>(arg), nullptr);
- server->StartShutDown();
-}
-
bool cmServerBase::StartServeThread()
{
ServeThreadRunning = true;
- uv_async_init(&Loop, &this->ShutdownSignal, __shutdownThread);
- this->ShutdownSignal.data = this;
uv_thread_create(&ServeThread, __start_thread, this);
return true;
}
+static void __shutdownThread(uv_async_t* arg)
+{
+ auto server = static_cast<cmServerBase*>(arg->data);
+ server->StartShutDown();
+}
+
bool cmServerBase::Serve(std::string* errorMessage)
{
#ifndef NDEBUG
@@ -443,14 +443,13 @@ bool cmServerBase::Serve(std::string* errorMessage)
errorMessage->clear();
- uv_signal_init(&Loop, &this->SIGINTHandler);
- uv_signal_init(&Loop, &this->SIGHUPHandler);
+ ShutdownSignal.init(Loop, __shutdownThread, this);
- this->SIGINTHandler.data = this;
- this->SIGHUPHandler.data = this;
+ SIGINTHandler.init(Loop, this);
+ SIGHUPHandler.init(Loop, this);
- uv_signal_start(&this->SIGINTHandler, &on_signal, SIGINT);
- uv_signal_start(&this->SIGHUPHandler, &on_signal, SIGHUP);
+ SIGINTHandler.start(&on_signal, SIGINT);
+ SIGHUPHandler.start(&on_signal, SIGHUP);
OnServeStart();
@@ -473,7 +472,6 @@ bool cmServerBase::Serve(std::string* errorMessage)
return false;
}
- ServeThreadRunning = false;
return true;
}
@@ -487,15 +485,9 @@ void cmServerBase::OnServeStart()
void cmServerBase::StartShutDown()
{
- if (!uv_is_closing(
- reinterpret_cast<const uv_handle_t*>(&this->SIGINTHandler))) {
- uv_signal_stop(&this->SIGINTHandler);
- }
-
- if (!uv_is_closing(
- reinterpret_cast<const uv_handle_t*>(&this->SIGHUPHandler))) {
- uv_signal_stop(&this->SIGHUPHandler);
- }
+ ShutdownSignal.reset();
+ SIGINTHandler.reset();
+ SIGHUPHandler.reset();
{
cm::unique_lock<cm::shared_mutex> lock(ConnectionsMutex);
@@ -519,20 +511,27 @@ cmServerBase::cmServerBase(cmConnection* connection)
{
auto err = uv_loop_init(&Loop);
(void)err;
+ Loop.data = this;
assert(err == 0);
AddNewConnection(connection);
}
-cmServerBase::~cmServerBase()
+void cmServerBase::Close()
{
+ if (Loop.data) {
+ if (ServeThreadRunning) {
+ this->ShutdownSignal.send();
+ uv_thread_join(&ServeThread);
+ }
- if (ServeThreadRunning) {
- uv_async_send(&this->ShutdownSignal);
- uv_thread_join(&ServeThread);
+ uv_loop_close(&Loop);
+ Loop.data = nullptr;
}
-
- uv_loop_close(&Loop);
+}
+cmServerBase::~cmServerBase()
+{
+ Close();
}
void cmServerBase::AddNewConnection(cmConnection* ownedConnection)
@@ -562,6 +561,6 @@ void cmServerBase::OnDisconnect(cmConnection* pConnection)
}
if (Connections.empty()) {
- StartShutDown();
+ this->ShutdownSignal.send();
}
}
diff --git a/Source/cmServer.h b/Source/cmServer.h
index 6e46f8c..ca37ce2 100644
--- a/Source/cmServer.h
+++ b/Source/cmServer.h
@@ -8,6 +8,8 @@
#include "cm_thread.hxx"
#include "cm_uv.h"
+#include "cmUVHandlePtr.h"
+
#include <memory> // IWYU pragma: keep
#include <string>
#include <vector>
@@ -58,7 +60,7 @@ public:
virtual bool OnSignal(int signum);
uv_loop_t* GetLoop();
-
+ void Close();
void OnDisconnect(cmConnection* pConnection);
protected:
@@ -67,7 +69,7 @@ protected:
bool ServeThreadRunning = false;
uv_thread_t ServeThread;
- uv_async_t ShutdownSignal;
+ cm::uv_async_ptr ShutdownSignal;
#ifndef NDEBUG
public:
// When the server starts it will mark down it's current thread ID,
@@ -80,8 +82,8 @@ protected:
uv_loop_t Loop;
- uv_signal_t SIGINTHandler;
- uv_signal_t SIGHUPHandler;
+ cm::uv_signal_ptr SIGINTHandler;
+ cm::uv_signal_ptr SIGHUPHandler;
};
class cmServer : public cmServerBase
diff --git a/Source/cmServerConnection.cxx b/Source/cmServerConnection.cxx
index 44af75f..78c8f06 100644
--- a/Source/cmServerConnection.cxx
+++ b/Source/cmServerConnection.cxx
@@ -5,6 +5,9 @@
#include "cmConfigure.h"
#include "cmServer.h"
#include "cmServerDictionary.h"
+#include "cm_uv.h"
+
+#include <algorithm>
#ifdef _WIN32
#include "io.h"
#else
@@ -18,36 +21,34 @@ cmStdIoConnection::cmStdIoConnection(
{
}
-void cmStdIoConnection::SetupStream(uv_stream_t*& stream, int file_id)
+cm::uv_stream_ptr cmStdIoConnection::SetupStream(int file_id)
{
- assert(stream == nullptr);
switch (uv_guess_handle(file_id)) {
case UV_TTY: {
- auto tty = new uv_tty_t();
- uv_tty_init(this->Server->GetLoop(), tty, file_id, file_id == 0);
+ cm::uv_tty_ptr tty;
+ tty.init(*this->Server->GetLoop(), file_id, file_id == 0,
+ static_cast<cmEventBasedConnection*>(this));
uv_tty_set_mode(tty, UV_TTY_MODE_NORMAL);
- stream = reinterpret_cast<uv_stream_t*>(tty);
- break;
+ return std::move(tty);
}
case UV_FILE:
if (file_id == 0) {
- return;
+ return nullptr;
}
// Intentional fallthrough; stdin can _not_ be treated as a named
// pipe, however stdout can be.
CM_FALLTHROUGH;
case UV_NAMED_PIPE: {
- auto pipe = new uv_pipe_t();
- uv_pipe_init(this->Server->GetLoop(), pipe, 0);
+ cm::uv_pipe_ptr pipe;
+ pipe.init(*this->Server->GetLoop(), 0,
+ static_cast<cmEventBasedConnection*>(this));
uv_pipe_open(pipe, file_id);
- stream = reinterpret_cast<uv_stream_t*>(pipe);
- break;
+ return std::move(pipe);
}
default:
assert(false && "Unable to determine stream type");
- return;
+ return nullptr;
}
- stream->data = static_cast<cmEventBasedConnection*>(this);
}
void cmStdIoConnection::SetServer(cmServerBase* s)
@@ -57,14 +58,14 @@ void cmStdIoConnection::SetServer(cmServerBase* s)
return;
}
- SetupStream(this->ReadStream, 0);
- SetupStream(this->WriteStream, 1);
+ this->ReadStream = SetupStream(0);
+ this->WriteStream = SetupStream(1);
}
void shutdown_connection(uv_prepare_t* prepare)
{
cmStdIoConnection* connection =
- reinterpret_cast<cmStdIoConnection*>(prepare->data);
+ static_cast<cmStdIoConnection*>(prepare->data);
if (!uv_is_closing(reinterpret_cast<uv_handle_t*>(prepare))) {
uv_close(reinterpret_cast<uv_handle_t*>(prepare),
@@ -76,7 +77,7 @@ void shutdown_connection(uv_prepare_t* prepare)
bool cmStdIoConnection::OnServeStart(std::string* pString)
{
Server->OnConnected(this);
- if (this->ReadStream) {
+ if (this->ReadStream.get()) {
uv_read_start(this->ReadStream, on_alloc_buffer, on_read);
} else if (uv_guess_handle(0) == UV_FILE) {
char buffer[1024];
@@ -94,44 +95,14 @@ bool cmStdIoConnection::OnServeStart(std::string* pString)
return cmConnection::OnServeStart(pString);
}
-void cmStdIoConnection::ShutdownStream(uv_stream_t*& stream)
-{
- if (!stream) {
- return;
- }
- switch (stream->type) {
- case UV_TTY: {
- assert(!uv_is_closing(reinterpret_cast<uv_handle_t*>(stream)));
- if (!uv_is_closing(reinterpret_cast<uv_handle_t*>(stream))) {
- uv_close(reinterpret_cast<uv_handle_t*>(stream),
- &on_close_delete<uv_tty_t>);
- }
- break;
- }
- case UV_FILE:
- case UV_NAMED_PIPE: {
- assert(!uv_is_closing(reinterpret_cast<uv_handle_t*>(stream)));
- if (!uv_is_closing(reinterpret_cast<uv_handle_t*>(stream))) {
- uv_close(reinterpret_cast<uv_handle_t*>(stream),
- &on_close_delete<uv_pipe_t>);
- }
- break;
- }
- default:
- assert(false && "Unable to determine stream type");
- }
-
- stream = nullptr;
-}
-
bool cmStdIoConnection::OnConnectionShuttingDown()
{
- if (ReadStream) {
+ if (ReadStream.get()) {
uv_read_stop(ReadStream);
+ ReadStream->data = nullptr;
}
- ShutdownStream(ReadStream);
- ShutdownStream(WriteStream);
+ this->ReadStream.reset();
cmEventBasedConnection::OnConnectionShuttingDown();
diff --git a/Source/cmServerConnection.h b/Source/cmServerConnection.h
index 4ca908d..a70edb4 100644
--- a/Source/cmServerConnection.h
+++ b/Source/cmServerConnection.h
@@ -8,7 +8,7 @@
#include "cmConnection.h"
#include "cmPipeConnection.h"
-#include "cm_uv.h"
+#include "cmUVHandlePtr.h"
class cmServerBase;
@@ -46,8 +46,8 @@ public:
bool OnServeStart(std::string* pString) override;
private:
- void SetupStream(uv_stream_t*& stream, int file_id);
- void ShutdownStream(uv_stream_t*& stream);
+ cm::uv_stream_ptr SetupStream(int file_id);
+ cm::uv_stream_ptr ReadStream;
};
/***
--
cgit v0.12