kapitan/IXWebSocket
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix DNSLookup memory leak (#422)

Browse Source 
* Fix memory leak with shared_ptr and -fsanitize=address

* Replace addrinfo* by shared_ptr

* fsanitize=address only on Linux

* Add USE_WS Linux CI test

* Remove fsanitize from the cmake files

* Remove USE_WS in linux test suite
This commit is contained in:
itytophile
2022-12-23 02:13:51 +01:00
committed by GitHub
parent a5d4911a16
commit 679ce519dd
5 changed files with 24 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
ixwebsocket/IXDNSLookup.cpp
View File

@@ -23,6 +23,7 @@
#include <chrono> #include <chrono>
#include <string.h> #include <string.h>
#include <thread> #include <thread>
#include <utility>
// mingw build quirks // mingw build quirks
#if defined(_WIN32) && defined(__GNUC__) #if defined(_WIN32) && defined(__GNUC__)
@@ -44,7 +45,7 @@ namespace ix
; ;
} }
struct addrinfo* DNSLookup::getAddrInfo(const std::string& hostname, DNSLookup::AddrInfoPtr DNSLookup::getAddrInfo(const std::string& hostname,
int port, int port,
std::string& errMsg) std::string& errMsg)
{ {
@@ -63,10 +64,10 @@ namespace ix
errMsg = gai_strerror(getaddrinfo_result); errMsg = gai_strerror(getaddrinfo_result);
res = nullptr; res = nullptr;
} }
return res; return AddrInfoPtr{ res, freeaddrinfo };
} }
struct addrinfo* DNSLookup::resolve(std::string& errMsg, DNSLookup::AddrInfoPtr DNSLookup::resolve(std::string& errMsg,
const CancellationRequest& isCancellationRequested, const CancellationRequest& isCancellationRequested,
bool cancellable) bool cancellable)
{ {
@@ -74,12 +75,7 @@ namespace ix
: resolveUnCancellable(errMsg, isCancellationRequested); : resolveUnCancellable(errMsg, isCancellationRequested);
} }
void DNSLookup::release(struct addrinfo* addr) DNSLookup::AddrInfoPtr DNSLookup::resolveUnCancellable(
{
freeaddrinfo(addr);
}
struct addrinfo* DNSLookup::resolveUnCancellable(
std::string& errMsg, const CancellationRequest& isCancellationRequested) std::string& errMsg, const CancellationRequest& isCancellationRequested)
{ {
errMsg = "no error"; errMsg = "no error";
@@ -94,7 +90,7 @@ namespace ix
return getAddrInfo(_hostname, _port, errMsg); return getAddrInfo(_hostname, _port, errMsg);
} }
struct addrinfo* DNSLookup::resolveCancellable( DNSLookup::AddrInfoPtr DNSLookup::resolveCancellable(
std::string& errMsg, const CancellationRequest& isCancellationRequested) std::string& errMsg, const CancellationRequest& isCancellationRequested)
{ {
errMsg = "no error"; errMsg = "no error";
@@ -157,7 +153,7 @@ namespace ix
// gone, so we use temporary variables (res) or we pass in by copy everything that // gone, so we use temporary variables (res) or we pass in by copy everything that
// getAddrInfo needs to work. // getAddrInfo needs to work.
std::string errMsg; std::string errMsg;
struct addrinfo* res = getAddrInfo(hostname, port, errMsg); auto res = getAddrInfo(hostname, port, errMsg);
if (auto lock = self.lock()) if (auto lock = self.lock())
{ {
@@ -181,13 +177,13 @@ namespace ix
return _errMsg; return _errMsg;
} }
void DNSLookup::setRes(struct addrinfo* addr) void DNSLookup::setRes(DNSLookup::AddrInfoPtr addr)
{ {
std::lock_guard<std::mutex> lock(_resMutex); std::lock_guard<std::mutex> lock(_resMutex);
_res = addr; _res = std::move(addr);
} }
struct addrinfo* DNSLookup::getRes() DNSLookup::AddrInfoPtr DNSLookup::getRes()
{ {
std::lock_guard<std::mutex> lock(_resMutex); std::lock_guard<std::mutex> lock(_resMutex);
return _res; return _res;

17
ixwebsocket/IXDNSLookup.h
View File

@@ -24,22 +24,21 @@ namespace ix
class DNSLookup : public std::enable_shared_from_this<DNSLookup> class DNSLookup : public std::enable_shared_from_this<DNSLookup>
{ {
public: public:
using AddrInfoPtr = std::shared_ptr<addrinfo>;
DNSLookup(const std::string& hostname, int port, int64_t wait = DNSLookup::kDefaultWait); DNSLookup(const std::string& hostname, int port, int64_t wait = DNSLookup::kDefaultWait);
~DNSLookup() = default; ~DNSLookup() = default;
struct addrinfo* resolve(std::string& errMsg, AddrInfoPtr resolve(std::string& errMsg,
const CancellationRequest& isCancellationRequested, const CancellationRequest& isCancellationRequested,
bool cancellable = true); bool cancellable = true);
void release(struct addrinfo* addr);
private: private:
struct addrinfo* resolveCancellable(std::string& errMsg, AddrInfoPtr resolveCancellable(std::string& errMsg,
const CancellationRequest& isCancellationRequested); const CancellationRequest& isCancellationRequested);
struct addrinfo* resolveUnCancellable(std::string& errMsg, AddrInfoPtr resolveUnCancellable(std::string& errMsg,
const CancellationRequest& isCancellationRequested); const CancellationRequest& isCancellationRequested);
static struct addrinfo* getAddrInfo(const std::string& hostname, AddrInfoPtr getAddrInfo(const std::string& hostname,
int port, int port,
std::string& errMsg); std::string& errMsg);
@@ -48,15 +47,15 @@ namespace ix
void setErrMsg(const std::string& errMsg); void setErrMsg(const std::string& errMsg);
const std::string& getErrMsg(); const std::string& getErrMsg();
void setRes(struct addrinfo* addr); void setRes(AddrInfoPtr addr);
struct addrinfo* getRes(); AddrInfoPtr getRes();
std::string _hostname; std::string _hostname;
int _port; int _port;
int64_t _wait; int64_t _wait;
const static int64_t kDefaultWait; const static int64_t kDefaultWait;
struct addrinfo* _res; AddrInfoPtr _res;
std::mutex _resMutex; std::mutex _resMutex;
std::string _errMsg; std::string _errMsg;

5
ixwebsocket/IXSocketConnect.cpp
View File

@@ -102,7 +102,7 @@ namespace ix
// First do DNS resolution // First do DNS resolution
// //
auto dnsLookup = std::make_shared<DNSLookup>(hostname, port); auto dnsLookup = std::make_shared<DNSLookup>(hostname, port);
struct addrinfo* res = dnsLookup->resolve(errMsg, isCancellationRequested); auto res = dnsLookup->resolve(errMsg, isCancellationRequested);
if (res == nullptr) if (res == nullptr)
{ {
return -1; return -1;
@@ -112,7 +112,7 @@ namespace ix
// iterate through the records to find a working peer // iterate through the records to find a working peer
struct addrinfo* address; struct addrinfo* address;
for (address = res; address != nullptr; address = address->ai_next) for (address = res.get(); address != nullptr; address = address->ai_next)
{ {
// //
// Second try to connect to the remote host // Second try to connect to the remote host
@@ -124,7 +124,6 @@ namespace ix
} }
} }
freeaddrinfo(res);
return sockfd; return sockfd;
} }

10
test/IXDNSLookupTest.cpp
View File

@@ -19,13 +19,9 @@ TEST_CASE("dns", "[net]")
auto dnsLookup = std::make_shared<DNSLookup>("www.google.com", 80); auto dnsLookup = std::make_shared<DNSLookup>("www.google.com", 80);
std::string errMsg; std::string errMsg;
struct addrinfo* res; auto res = dnsLookup->resolve(errMsg, [] { return false; });
res = dnsLookup->resolve(errMsg, [] { return false; });
std::cerr << "Error message: " << errMsg << std::endl; std::cerr << "Error message: " << errMsg << std::endl;
REQUIRE(res != nullptr); REQUIRE(res != nullptr);
dnsLookup->release(res);
} }
SECTION("Test resolving a non-existing hostname") SECTION("Test resolving a non-existing hostname")
@@ -33,7 +29,7 @@ TEST_CASE("dns", "[net]")
auto dnsLookup = std::make_shared<DNSLookup>("wwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwww", 80); auto dnsLookup = std::make_shared<DNSLookup>("wwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwww", 80);
std::string errMsg; std::string errMsg;
struct addrinfo* res = dnsLookup->resolve(errMsg, [] { return false; }); auto res = dnsLookup->resolve(errMsg, [] { return false; });
std::cerr << "Error message: " << errMsg << std::endl; std::cerr << "Error message: " << errMsg << std::endl;
REQUIRE(res == nullptr); REQUIRE(res == nullptr);
} }
@@ -44,7 +40,7 @@ TEST_CASE("dns", "[net]")
std::string errMsg; std::string errMsg;
// The callback returning true means we are requesting cancellation // The callback returning true means we are requesting cancellation
struct addrinfo* res = dnsLookup->resolve(errMsg, [] { return true; }); auto res = dnsLookup->resolve(errMsg, [] { return true; });
std::cerr << "Error message: " << errMsg << std::endl; std::cerr << "Error message: " << errMsg << std::endl;
REQUIRE(res == nullptr); REQUIRE(res == nullptr);
} }

3
ws/ws.cpp
View File

@@ -892,9 +892,8 @@ namespace ix
auto dnsLookup = std::make_shared<DNSLookup>(hostname, 80); auto dnsLookup = std::make_shared<DNSLookup>(hostname, 80);
std::string errMsg; std::string errMsg;
struct addrinfo* res;
res = dnsLookup->resolve(errMsg, [] { return false; }); auto res = dnsLookup->resolve(errMsg, [] { return false; });
auto addr = res->ai_addr; auto addr = res->ai_addr;