[checkmk-commits] 3143 FIX windows agent: fixed some minor memory and handle leaks

Module: check_mk Branch: master Commit: 9184cff2c6094dd937e998ede7c5e6ceda3f650e URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=9184cff2c6094d… Author: Sebastian Herbord &lt;sh(a)mathias-kettner.de&gt; Date: Wed Jun 15 14:34:41 2016 +0200 3143 FIX windows agent: fixed some minor memory and handle leaks --- .werks/3143 | 10 ++++++++++ ChangeLog | 1 + agents/windows/ExternalCmd.cc | 2 ++ agents/windows/ListenSocket.cc | 18 +++++++++--------- agents/windows/SettingsCollector.h | 10 ++++------ agents/windows/build_version | 2 +- agents/windows/check_mk_agent-64.exe | Bin 1270784 -> 1270784 bytes agents/windows/check_mk_agent-64.unversioned.exe | Bin 1270784 -> 1270784 bytes agents/windows/check_mk_agent.cc | 17 ++++++++++++++--- agents/windows/check_mk_agent.exe | Bin 1238528 -> 1239552 bytes agents/windows/check_mk_agent.msi | Bin 2778112 -> 248320 bytes agents/windows/check_mk_agent.unversioned.exe | Bin 1238528 -> 1239552 bytes agents/windows/install_agent-64.exe | Bin 566235 -> 566335 bytes agents/windows/install_agent.exe | Bin 560412 -> 560642 bytes agents/windows/wmiHelper.cc | 2 ++ agents/windows/wmiHelper.h | 12 +++++------- 16 files changed, 48 insertions(+), 26 deletions(-) diff --git a/.werks/3143 b/.werks/3143 new file mode 100644 index 0000000..d351b1c --- /dev/null +++ b/.werks/3143 @@ -0,0 +1,10 @@ +Title: windows agent: fixed some minor memory and handle leaks +Level: 1 +Component: checks +Class: fix +Compatible: compat +State: unknown +Version: 1.4.0i1 +Date: 1465993766 + + diff --git a/ChangeLog b/ChangeLog index 1956125..dde3338 100644 --- a/ChangeLog +++ b/ChangeLog @@ -271,6 +271,7 @@ * 3142 FIX: windows agent: fixed rare crash when wmi query for uptime fails... * 3637 FIX: ipmi_sensors: fixed sensor discovery if sensor is unknown and perfdata handling * 3545 FIX: netapp_api_vs_traffic: fixed crash when NFSv4.1 information was available + * 3143 FIX: windows agent: fixed some minor memory and handle leaks Multisite: * 3187 notification view: new filter for log command via regex diff --git a/agents/windows/ExternalCmd.cc b/agents/windows/ExternalCmd.cc index 8dbab5e..7f32358 100644 --- a/agents/windows/ExternalCmd.cc +++ b/agents/windows/ExternalCmd.cc @@ -97,12 +97,14 @@ ExternalCmd::~ExternalCmd() { ::CloseHandle(_stderr); if (_job_object != INVALID_HANDLE_VALUE) { ::TerminateJobObject(_job_object, 1); + ::CloseHandle(_job_object); } ::CloseHandle(_process); } void ExternalCmd::terminateJob(DWORD exit_code) { ::TerminateJobObject(_job_object, exit_code); + ::CloseHandle(_job_object); _job_object = INVALID_HANDLE_VALUE; } diff --git a/agents/windows/ListenSocket.cc b/agents/windows/ListenSocket.cc index c520df2..2d07e6e 100644 --- a/agents/windows/ListenSocket.cc +++ b/agents/windows/ListenSocket.cc @@ -27,6 +27,7 @@ #include <ws2ipdef.h> #include <cassert> #include <cstring> +#include <memory> #include "logging.h" #include "stringutil.h" @@ -122,13 +123,13 @@ SOCKET ListenSocket::init_listen_socket(int port) { SOCKET s = RemoveSocketInheritance(tmp_s); int addr_size = 0; - SOCKADDR *addr = create_sockaddr(&addr_size); + std::unique_ptr<SOCKADDR> addr(create_sockaddr(&addr_size)); int optval = 1; setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&optval, sizeof(optval)); if (_use_ipv6) { - ((SOCKADDR_IN6 *)addr)->sin6_port = htons(port); + ((SOCKADDR_IN6 *)addr.get())->sin6_port = htons(port); int v6only = 0; if (setsockopt(s, IPPROTO_IPV6, IPV6_V6ONLY, (char *)&v6only, @@ -137,11 +138,11 @@ SOCKET ListenSocket::init_listen_socket(int port) { _supports_ipv4 = false; } } else { - ((SOCKADDR_IN *)addr)->sin_port = htons(port); - ((SOCKADDR_IN *)addr)->sin_addr.s_addr = ADDR_ANY; + ((SOCKADDR_IN *)addr.get())->sin_port = htons(port); + ((SOCKADDR_IN *)addr.get())->sin_addr.s_addr = ADDR_ANY; } - if (SOCKET_ERROR == bind(s, addr, addr_size)) { + if (SOCKET_ERROR == bind(s, addr.get(), addr_size)) { int error_id = ::WSAGetLastError(); fprintf(stderr, "Cannot bind socket to port %d: %s (%d)\n", port, get_win_error_as_string(error_id).c_str(), error_id); @@ -221,12 +222,11 @@ SOCKET ListenSocket::acceptConnection() { // never return while (1 == select(1, &fds, NULL, NULL, &timeout)) { int addr_len = 0; - sockaddr *remote_addr = create_sockaddr(&addr_len); - connection = accept(_socket, remote_addr, &addr_len); + std::unique_ptr<sockaddr> remote_addr(create_sockaddr(&addr_len)); + connection = accept(_socket, remote_addr.get(), &addr_len); connection = RemoveSocketInheritance(connection); if (connection != INVALID_SOCKET) { - bool allowed = check_only_from(remote_addr); - delete remote_addr; + bool allowed = check_only_from(remote_addr.get()); if (allowed) { return connection; diff --git a/agents/windows/SettingsCollector.h b/agents/windows/SettingsCollector.h index da2d449..0464fa8 100644 --- a/agents/windows/SettingsCollector.h +++ b/agents/windows/SettingsCollector.h @@ -171,12 +171,10 @@ public: const ContainerT &operator*() const { return _values; } public: // only valid with a grouping adder. it's important to understand that - // due to how templates in C++ work, - // these functions are not compiled for template-instantiations where they - // aren't used so - // even though they wouldn't comple with non-grouping adders, this is not a - // problem. - + // due to how templates in C++ work, these functions are not compiled + // for template-instantiations where they aren't used so even though + // they wouldn't comple with non-grouping adders, this is not a + // problem. void setGroupFunction(typename AddMode::PriorityAppendGrouped< ContainerT>::GroupFunction function) { _add_mode.setGroupFunction(function); diff --git a/agents/windows/build_version b/agents/windows/build_version index ca06b0e..4f5ed40 100644 --- a/agents/windows/build_version +++ b/agents/windows/build_version @@ -1 +1 @@ -1761 +1763 diff --git a/agents/windows/check_mk_agent-64.exe b/agents/windows/check_mk_agent-64.exe index 94c67ad..6cfd91a 100755 Binary files a/agents/windows/check_mk_agent-64.exe and b/agents/windows/check_mk_agent-64.exe differ diff --git a/agents/windows/check_mk_agent-64.unversioned.exe b/agents/windows/check_mk_agent-64.unversioned.exe index 5a2a8fa..5c6c901 100755 Binary files a/agents/windows/check_mk_agent-64.unversioned.exe and b/agents/windows/check_mk_agent-64.unversioned.exe differ diff --git a/agents/windows/check_mk_agent.cc b/agents/windows/check_mk_agent.cc index a12206c..a98e9a1 100644 --- a/agents/windows/check_mk_agent.cc +++ b/agents/windows/check_mk_agent.cc @@ -1203,10 +1203,19 @@ void update_script_statistics() { // Remove missing files from list void cleanup_logwatch_textfiles() { logwatch_textfiles_t &textfiles = g_config->logwatchTextfiles(); - textfiles.erase( + + // remove_if puts the missing textfiles to the end of the list, it doesn't + // actually remove anything + auto first_missing = std::remove_if(textfiles.begin(), textfiles.end(), - [](logwatch_textfile *file) { return file->missing; }), - textfiles.end()); + [](logwatch_textfile *file) { return file->missing; }); + + for (logwatch_textfiles_t::iterator iter = first_missing; + iter != textfiles.end(); ++iter) { + delete *iter; + } + + textfiles.erase(first_missing, textfiles.end()); } // Called on program exit @@ -2910,8 +2919,10 @@ void stop_threads() { } it_cont++; } + WaitForMultipleObjects(active_thread_count, hThreadArray, TRUE, 5000); TerminateJobObject(g_workers_job_object, 0); + ::CloseHandle(g_workers_job_object); } // .----------------------------------------------------------------------. diff --git a/agents/windows/check_mk_agent.exe b/agents/windows/check_mk_agent.exe index 75bac1e..60e0615 100755 Binary files a/agents/windows/check_mk_agent.exe and b/agents/windows/check_mk_agent.exe differ diff --git a/agents/windows/check_mk_agent.msi b/agents/windows/check_mk_agent.msi index 31ce86c..b59d1c6 100755 Binary files a/agents/windows/check_mk_agent.msi and b/agents/windows/check_mk_agent.msi differ diff --git a/agents/windows/check_mk_agent.unversioned.exe b/agents/windows/check_mk_agent.unversioned.exe index 8e71be7..77a1c2a 100755 Binary files a/agents/windows/check_mk_agent.unversioned.exe and b/agents/windows/check_mk_agent.unversioned.exe differ diff --git a/agents/windows/install_agent-64.exe b/agents/windows/install_agent-64.exe index d9bcddf..2ee5ca7 100755 Binary files a/agents/windows/install_agent-64.exe and b/agents/windows/install_agent-64.exe differ diff --git a/agents/windows/install_agent.exe b/agents/windows/install_agent.exe index 6e50880..e07f73c 100755 Binary files a/agents/windows/install_agent.exe and b/agents/windows/install_agent.exe differ diff --git a/agents/windows/wmiHelper.cc b/agents/windows/wmiHelper.cc index 3fbe910..2baa7a4 100644 --- a/agents/windows/wmiHelper.cc +++ b/agents/windows/wmiHelper.cc @@ -348,6 +348,8 @@ public: private: COMManager() { + // Dr.Memory reports a memory leak here, despite the fact CoUninitialize + // does get called. Am I doing something wrong? HRESULT res = CoInitializeEx(0, COINIT_MULTITHREADED); if (FAILED(res)) { throw ComException("Failed to initialize COM", res); diff --git a/agents/windows/wmiHelper.h b/agents/windows/wmiHelper.h index 9ecb5fd..5abe581 100644 --- a/agents/windows/wmiHelper.h +++ b/agents/windows/wmiHelper.h @@ -137,13 +137,11 @@ public: std::vector<std::wstring> names() const; - // proceed to the next element. - // returns true on success, false if there are no more elements. An - // exception - // is thrown if an error happens (i.e. timeout in the query). - // unless true is returned, the current element is not changed, to once the - // end - // of the result has been reached, the iterator stays there. + // proceed to the next element. returns true on success, false if there are + // no more elements. An exception is thrown if an error happens (i.e. + // timeout in the query). unless true is returned, the current element is + // not changed, to once the end of the result has been reached, the iterator + // stays there. bool next(); // return true if this is a valid result. Please note that