From b140a1c3400f7d6341a401461b38a4566aea24cd Mon Sep 17 00:00:00 2001 From: Adrien Destugues Date: Tue, 21 Nov 2017 22:11:03 +0100 Subject: [PATCH] NetworkAddressResolver: cache needs to be locked It is not a good idea to have a thread get an address from the request cache, while another thread is deleting said address as the cache has grown too large. Add a lock around the cache access to make it safe. --- headers/os/net/NetworkAddressResolver.h | 4 +++- .../network/libnetapi/NetworkAddressResolver.cpp | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/headers/os/net/NetworkAddressResolver.h b/headers/os/net/NetworkAddressResolver.h index c5bd627bcb..9c42d38722 100644 --- a/headers/os/net/NetworkAddressResolver.h +++ b/headers/os/net/NetworkAddressResolver.h @@ -1,11 +1,12 @@ /* - * Copyright 2010, Haiku, Inc. All Rights Reserved. + * Copyright 2010-2017, Haiku, Inc. All Rights Reserved. * Distributed under the terms of the MIT License. */ #ifndef _NETWORK_ADDRESS_RESOLVER_H #define _NETWORK_ADDRESS_RESOLVER_H +#include #include #include #include @@ -103,6 +104,7 @@ private: BReference fResolver; }; + static BLocker sCacheLock; static BObjectList sCacheMap; }; diff --git a/src/kits/network/libnetapi/NetworkAddressResolver.cpp b/src/kits/network/libnetapi/NetworkAddressResolver.cpp index e7e61eac4e..683445b11a 100644 --- a/src/kits/network/libnetapi/NetworkAddressResolver.cpp +++ b/src/kits/network/libnetapi/NetworkAddressResolver.cpp @@ -1,5 +1,6 @@ /* * Copyright 2010-2011, Axel Dörfler, axeld@pinc-software.de. + * Copyright 2015-2017, Adrien Destugues, pulkomandy@pulkomandy.tk. * Distributed under the terms of the MIT License. */ @@ -9,6 +10,7 @@ #include #include +#include #include @@ -296,6 +298,8 @@ BNetworkAddressResolver::Resolve(int family, const char* address, BNetworkAddressResolver::Resolve(int family, const char* address, const char* service, uint32 flags) { + BAutolock locker(&sCacheLock); + // TODO it may be faster to use an hash map to have faster lookup of the // cache. However, we also need to access the cache by LRU, and for that // a doubly-linked list is better. We should have these two share the same @@ -312,17 +316,23 @@ BNetworkAddressResolver::Resolve(int family, const char* address, } } + // Cache miss! Unlock the cache while we perform the costly address + // resolution + locker.Unlock(); + BNetworkAddressResolver* resolver = new(std::nothrow) BNetworkAddressResolver(family, address, service, flags); if (resolver != NULL && resolver->InitCheck() == B_OK) { + CacheEntry* entry = new(std::nothrow) CacheEntry(family, address, + service, flags, resolver); + + locker.Lock(); // TODO adjust capacity. Chrome uses 256 entries with a timeout of // 1 minute, IE uses 1000 entries with a timeout of 30 seconds. if (sCacheMap.CountItems() > 255) delete sCacheMap.RemoveItemAt(0); - CacheEntry* entry = new(std::nothrow) CacheEntry(family, address, - service, flags, resolver); if (entry) sCacheMap.AddItem(entry, sCacheMap.CountItems()); } @@ -330,6 +340,6 @@ BNetworkAddressResolver::Resolve(int family, const char* address, return BReference(resolver, true); } - +BLocker BNetworkAddressResolver::sCacheLock("DNS cache"); BObjectList BNetworkAddressResolver::sCacheMap;