[checkmk-commits] Windows agent: use RAII for socket handles

Module: check_mk Branch: master Commit: 6837286c3a58d87740d07650ade155bee58168fc URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=6837286c3a58d8… Author: Jukka Aro &lt;ja(a)mathias-kettner.de&gt; Date: Mon Feb 26 13:07:39 2018 +0100 Windows agent: use RAII for socket handles --- agents/windows/ListenSocket.cc | 28 ++++++++++------------------ agents/windows/ListenSocket.h | 16 +++++++++++++--- agents/windows/build_version | 2 +- agents/windows/check_mk_agent.cc | 35 ++++++++++++++++------------------- 4 files changed, 40 insertions(+), 41 deletions(-) diff --git a/agents/windows/ListenSocket.cc b/agents/windows/ListenSocket.cc index ea27ad9..122a484 100644 --- a/agents/windows/ListenSocket.cc +++ b/agents/windows/ListenSocket.cc @@ -40,12 +40,10 @@ ListenSocket::ListenSocket(int port, const only_from_t &source_whitelist, : _logger(logger) , _winapi(winapi) , _use_ipv6(supportIPV6) - , _socket(init_listen_socket(port)) + , _socket(init_listen_socket(port), winapi) , _source_whitelist(source_whitelist) , _supports_ipv4(true) {} -ListenSocket::~ListenSocket() { _winapi.closesocket(_socket); } - bool ListenSocket::supportsIPV4() const { return _supports_ipv4; } bool ListenSocket::supportsIPV6() const { return _use_ipv6; } @@ -206,13 +204,11 @@ sockaddr *ListenSocket::create_sockaddr(int *addr_len) const { return result; } -SOCKET ListenSocket::acceptConnection() const { - SOCKET connection; +SocketHandle ListenSocket::acceptConnection() const { // Loop forever. - fd_set fds; FD_ZERO(&fds); - FD_SET(_socket, &fds); + FD_SET(_socket.get(), &fds); struct timeval timeout; timeout.tv_sec = 0; timeout.tv_usec = 500000; @@ -222,17 +218,13 @@ SOCKET ListenSocket::acceptConnection() const { while (1 == _winapi.select(1, &fds, NULL, NULL, &timeout)) { int addr_len = 0; std::unique_ptr<sockaddr> remote_addr(create_sockaddr(&addr_len)); - connection = _winapi.accept(_socket, remote_addr.get(), &addr_len); - connection = RemoveSocketInheritance(connection); - if (connection != INVALID_SOCKET) { - bool allowed = check_only_from(remote_addr.get()); - - if (allowed) { - return connection; - } else { - _winapi.closesocket(connection); - } + SOCKET rawSocket = + _winapi.accept(_socket.get(), remote_addr.get(), &addr_len); + SocketHandle connection(RemoveSocketInheritance(rawSocket), _winapi); + if (connection && check_only_from(remote_addr.get())) { + return connection; } } - return 0; + + return SocketHandle(_winapi); } diff --git a/agents/windows/ListenSocket.h b/agents/windows/ListenSocket.h index e9c34f5..af30004 100644 --- a/agents/windows/ListenSocket.h +++ b/agents/windows/ListenSocket.h @@ -31,16 +31,26 @@ class Logger; class WinApiAdaptor; +struct SocketHandleTraits { + using HandleT = SOCKET; + static HandleT invalidValue() { return INVALID_SOCKET; } + + static void closeHandle(HandleT value, const WinApiAdaptor &winapi) { + winapi.closesocket(value); + } +}; + +using SocketHandle = WrappedHandle<SocketHandleTraits>; + class ListenSocket { public: ListenSocket(int port, const only_from_t &source_whitelist, bool supportIPV6, Logger *logger, const WinApiAdaptor &winapi); - ~ListenSocket(); bool supportsIPV4() const; bool supportsIPV6() const; - SOCKET acceptConnection() const; + SocketHandle acceptConnection() const; sockaddr_storage address(SOCKET connection) const; @@ -56,7 +66,7 @@ private: Logger *_logger; const WinApiAdaptor &_winapi; const bool _use_ipv6; - SOCKET _socket; + SocketHandle _socket; const only_from_t _source_whitelist; bool _supports_ipv4; }; diff --git a/agents/windows/build_version b/agents/windows/build_version index ea0965b..fc9dc43 100644 --- a/agents/windows/build_version +++ b/agents/windows/build_version @@ -1 +1 @@ -3120 +3121 diff --git a/agents/windows/check_mk_agent.cc b/agents/windows/check_mk_agent.cc index 7349668..86096bb 100644 --- a/agents/windows/check_mk_agent.cc +++ b/agents/windows/check_mk_agent.cc @@ -566,7 +566,7 @@ DWORD WINAPI realtime_check_func(void *data_in) { try { sockaddr_storage current_address; std::string current_ip; - SOCKET current_socket = INVALID_SOCKET; + SocketHandle current_socket(s_winapi); std::unique_ptr<BufferedSocketProxy> out; if (*s_config->encrypted_rt) { @@ -596,10 +596,9 @@ DWORD WINAPI realtime_check_func(void *data_in) { if (data->new_request) { data->new_request = false; // (re-)establish connection if necessary - if (current_socket != INVALID_SOCKET) { - s_winapi.closesocket(current_socket); - out->setSocket(INVALID_SOCKET); - } + current_socket.reset(); + out->setSocket(INVALID_SOCKET); + current_address = data->last_address; if (current_address.ss_family != 0) { int sockaddr_size = 0; @@ -651,27 +650,27 @@ DWORD WINAPI realtime_check_func(void *data_in) { } current_ip = ListenSocket::readableIP(&current_address); - current_socket = s_winapi.socket( - current_address.ss_family, SOCK_DGRAM, IPPROTO_UDP); - if (current_socket == INVALID_SOCKET) { + current_socket.reset( + s_winapi.socket(current_address.ss_family, + SOCK_DGRAM, IPPROTO_UDP)); + if (!current_socket) { Emergency(logger) << "failed to establish socket: " << s_winapi.WSAGetLastError(); return 1; } - if (s_winapi.connect(current_socket, + if (s_winapi.connect(current_socket.get(), (const sockaddr *)&current_address, sockaddr_size) == SOCKET_ERROR) { Emergency(logger) << "failed to connect: " << s_winapi.WSAGetLastError(); - s_winapi.closesocket(current_socket); - current_socket = INVALID_SOCKET; + current_socket.reset(); } - out->setSocket(current_socket); + out->setSocket(current_socket.get()); } } // send data - if (current_socket != INVALID_SOCKET) { + if (current_socket) { // send data s_winapi.SetEnvironmentVariable("REMOTE_HOST", current_ip.c_str()); @@ -689,7 +688,6 @@ DWORD WINAPI realtime_check_func(void *data_in) { } } } - s_winapi.closesocket(current_socket); return 0; } catch (const std::exception &e) { @@ -744,20 +742,20 @@ void do_adhoc(const Environment &env) { } while (!g_should_terminate) { - SOCKET connection = sock.acceptConnection(); + SocketHandle connection = std::move(sock.acceptConnection()); if (connection) { - out->setSocket(connection); + out->setSocket(connection.get()); if (*s_config->encrypted) { out->writeBinary(RT_PROTOCOL_VERSION, 2); } - std::string ip_hr = sock.readableIP(connection); + std::string ip_hr = sock.readableIP(connection.get()); Debug(logger) << "Accepted client connection from " << ip_hr << "."; { // limit lifetime of mutex lock lock_guard<mutex>(thread_data.mutex); thread_data.new_request = true; - thread_data.last_address = sock.address(connection); + thread_data.last_address = sock.address(connection.get()); thread_data.push_until = time(NULL) + *s_config->realtime_timeout; } @@ -770,7 +768,6 @@ void do_adhoc(const Environment &env) { Alert(Logger::getLogger("winagent")) << "unhandled exception: " << e.what(); } - s_winapi.closesocket(connection); } }