From 1e9b7d3ceb882d3feb59324b6e55d40cc795576b Mon Sep 17 00:00:00 2001 From: Justin Berger 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(stream->data); + auto conn = static_cast(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(stream->data); + auto conn = static_cast(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(ds)); memcpy(req->buf.base, data.c_str(), ds); - uv_write(reinterpret_cast(req), - static_cast(this->WriteStream), &req->buf, 1, + uv_write(reinterpret_cast(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 @@ -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 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 + #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(rejectPipe)); - uv_close(reinterpret_cast(rejectPipe), - &on_close_delete); return; } - this->ClientPipe = new uv_pipe_t(); - uv_pipe_init(this->Server->GetLoop(), this->ClientPipe, 0); - this->ClientPipe->data = static_cast(this); - auto client = reinterpret_cast(this->ClientPipe); - if (uv_accept(server, client) != 0) { - uv_close(reinterpret_cast(client), - &on_close_delete); - this->ClientPipe = nullptr; + cm::uv_pipe_ptr ClientPipe; + ClientPipe.init(*this->Server->GetLoop(), 0, + static_cast(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(this); + this->ServerPipe.init(*this->Server->GetLoop(), 0, + static_cast(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(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(this->ClientPipe), - &on_close_delete); + if (this->WriteStream.get()) { this->WriteStream->data = nullptr; } - uv_close(reinterpret_cast(this->ServerPipe), - &on_close_delete); - 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 #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(signal->data); + auto conn = static_cast(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(arg); + auto server = static_cast(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(arg->data); - on_walk_to_shutdown(reinterpret_cast(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(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(&this->SIGINTHandler))) { - uv_signal_stop(&this->SIGINTHandler); - } - - if (!uv_is_closing( - reinterpret_cast(&this->SIGHUPHandler))) { - uv_signal_stop(&this->SIGHUPHandler); - } + ShutdownSignal.reset(); + SIGINTHandler.reset(); + SIGHUPHandler.reset(); { cm::unique_lock 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 // IWYU pragma: keep #include #include @@ -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 #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(this)); uv_tty_set_mode(tty, UV_TTY_MODE_NORMAL); - stream = reinterpret_cast(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(this)); uv_pipe_open(pipe, file_id); - stream = reinterpret_cast(pipe); - break; + return std::move(pipe); } default: assert(false && "Unable to determine stream type"); - return; + return nullptr; } - stream->data = static_cast(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(prepare->data); + static_cast(prepare->data); if (!uv_is_closing(reinterpret_cast(prepare))) { uv_close(reinterpret_cast(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(stream))); - if (!uv_is_closing(reinterpret_cast(stream))) { - uv_close(reinterpret_cast(stream), - &on_close_delete); - } - break; - } - case UV_FILE: - case UV_NAMED_PIPE: { - assert(!uv_is_closing(reinterpret_cast(stream))); - if (!uv_is_closing(reinterpret_cast(stream))) { - uv_close(reinterpret_cast(stream), - &on_close_delete); - } - 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