Archives/lokinet
2
0
Fork 0
mirror of https://github.com/oxen-io/lokinet.git synced 2024-11-17 15:25:35 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #472 from michael-loki/fix_catalog_test_failure

Browse Source 
Fix Catalog.Iterator test failure
This commit is contained in:
Jeff 2019-03-29 19:08:29 -04:00 committed by GitHub
commit 289de170ef
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 53 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CMakeLists.txt
View File

@ -190,7 +190,7 @@ if(NOT DEBIAN)
endif(NOT DEBIAN) endif(NOT DEBIAN)
if(ASAN) if(ASAN)
set(DEBUG_FLAGS ${DEBUG_FLAGS} -fsanitize=address -fno-omit-frame-pointer) set(DEBUG_FLAGS ${DEBUG_FLAGS} -fsanitize=undefined -fno-omit-frame-pointer)
set(OPTIMIZE_FLAGS "-O0") set(OPTIMIZE_FLAGS "-O0")
endif(ASAN) endif(ASAN)

48
llarp/util/object.hpp
View File

@ -105,22 +105,25 @@ namespace llarp
template < typename Value > template < typename Value >
class Catalog class Catalog
{ {
static constexpr int32_t INDEX_MASK = 0X007FFFFF; enum
static constexpr int32_t BUSY_INDICATOR = 0x00800000; {
static constexpr int32_t GENERATION_INC = 0x01000000; INDEX_MASK = 0X007FFFFF,
static constexpr int32_t GENERATION_MASK = 0XFF000000; BUSY_INDICATOR = 0x00800000,
GENERATION_INC = 0x01000000,
GENERATION_MASK = 0XFF000000
};
struct Node struct Node
{ {
typedef union { union Payload {
Buffer< Value > m_buffer; Buffer< Value > m_buffer;
Node* m_next; Node* m_next;
} Payload; };
Payload m_payload; Payload m_payload;
int32_t m_handle; int32_t m_handle;
}; };
std::vector< Node* > m_nodes; std::vector< Node* > m_nodes GUARDED_BY(m_mutex);
Node* m_next; Node* m_next;
std::atomic_size_t m_size; std::atomic_size_t m_size;
@ -146,11 +149,11 @@ namespace llarp
} }
Node* Node*
findNode(int32_t handle) const findNode(int32_t handle) const SHARED_LOCKS_REQUIRED(m_mutex)
{ {
int32_t index = handle & INDEX_MASK; int32_t index = handle & INDEX_MASK;
if(0 > index || index >= (int32_t)m_nodes.size() if(0 > index || index >= static_cast< int32_t >(m_nodes.size())
|| !(handle & BUSY_INDICATOR)) || !(handle & BUSY_INDICATOR))
{ {
return nullptr; return nullptr;
@ -190,11 +193,11 @@ namespace llarp
{ {
assert(m_nodes.size() < BUSY_INDICATOR); assert(m_nodes.size() < BUSY_INDICATOR);
node = static_cast< Node* >(operator new(sizeof(Node))); node = new Node;
guard.manageNode(node, true); guard.manageNode(node, true);
m_nodes.push_back(node); m_nodes.push_back(node);
node->m_handle = static_cast< int32_t >(m_nodes.size()) - 1; node->m_handle = static_cast< int32_t >(m_nodes.size() - 1);
guard.manageNode(node, false); guard.manageNode(node, false);
} }
@ -282,7 +285,7 @@ namespace llarp
absl::optional< Value > absl::optional< Value >
find(int32_t handle) find(int32_t handle)
{ {
util::Lock l(&m_mutex); absl::ReaderMutexLock l(&m_mutex);
Node* node = findNode(handle); Node* node = findNode(handle);
if(!node) if(!node)
@ -305,43 +308,48 @@ namespace llarp
}; };
template < typename Value > template < typename Value >
class CatalogIterator class SCOPED_LOCKABLE CatalogIterator
{ {
const Catalog< Value >* m_catalog; const Catalog< Value >* m_catalog;
size_t m_index; size_t m_index;
CatalogIterator(const CatalogIterator&) = delete;
CatalogIterator&
operator=(const CatalogIterator&) = delete;
public: public:
CatalogIterator(const Catalog< Value >& catalog) explicit CatalogIterator(const Catalog< Value >* catalog)
: m_catalog(&catalog), m_index(-1) SHARED_LOCK_FUNCTION(m_catalog->m_mutex)
: m_catalog(catalog), m_index(-1)
{ {
m_catalog->m_mutex.ReaderLock(); m_catalog->m_mutex.ReaderLock();
operator++(); operator++();
} }
~CatalogIterator() ~CatalogIterator() UNLOCK_FUNCTION()
{ {
m_catalog->m_mutex.ReaderUnlock(); m_catalog->m_mutex.ReaderUnlock();
} }
void void
operator++() operator++() NO_THREAD_SAFETY_ANALYSIS
{ {
m_index++; m_index++;
while(m_index < m_catalog->m_nodes.size() while(m_index < m_catalog->m_nodes.size()
&& !(m_catalog->m_nodes[m_index]->m_handle && !(m_catalog->m_nodes[m_index]->m_handle
& Catalog< Value >::BUSY_INDICATOR)) & Catalog< Value >::BUSY_INDICATOR))
{ {
++m_index; m_index++;
} }
} }
explicit operator bool() const explicit operator bool() const NO_THREAD_SAFETY_ANALYSIS
{ {
return m_index < m_catalog->m_nodes.size(); return m_index < m_catalog->m_nodes.size();
} }
std::pair< int32_t, Value > std::pair< int32_t, Value >
operator()() const operator()() const NO_THREAD_SAFETY_ANALYSIS
{ {
auto* node = m_catalog->m_nodes[m_index]; auto* node = m_catalog->m_nodes[m_index];
return {node->m_handle, *Catalog< Value >::getValue(node)}; return {node->m_handle, *Catalog< Value >::getValue(node)};

44
test/util/test_llarp_util_object.cpp
View File

@ -28,8 +28,8 @@ TEST(Catalog, smoke)
{ {
const double value1 = 1.0; const double value1 = 1.0;
const double value2 = 2.0; const double value2 = 2.0;
int handle1 = -1; int32_t handle1 = -1;
int handle2 = -1; int32_t handle2 = -1;
Catalog< double > catalog; Catalog< double > catalog;
@ -61,11 +61,12 @@ TEST(Catalog, Iterator)
{ {
static constexpr size_t THREAD_COUNT = 10; static constexpr size_t THREAD_COUNT = 10;
static constexpr size_t ITERATION_COUNT = 1000; static constexpr size_t ITERATION_COUNT = 1000;
static constexpr int32_t MAX = std::numeric_limits< int32_t >::max();
std::array< std::thread, THREAD_COUNT + 3 > threads; std::array< std::thread, THREAD_COUNT + 3 > threads;
using llarp::util::Barrier; using llarp::util::Barrier;
using Iterator = CatalogIterator< int >; using Iterator = CatalogIterator< int32_t >;
using Cat = Catalog< int >; using Cat = Catalog< int32_t >;
Barrier barrier(THREAD_COUNT + 3); Barrier barrier(THREAD_COUNT + 3);
Cat catalog; Cat catalog;
@ -74,22 +75,22 @@ TEST(Catalog, Iterator)
for(size_t i = 0; i < THREAD_COUNT; ++i) for(size_t i = 0; i < THREAD_COUNT; ++i)
{ {
threads[i] = std::thread( threads[i] = std::thread(
[](Barrier *barrier, Cat *catalog, int id) { [](Barrier *barrier, Cat *catalog, int32_t id) {
barrier->Block(); barrier->Block();
for(size_t i = 0; i < ITERATION_COUNT; ++i) for(size_t i = 0; i < ITERATION_COUNT; ++i)
{ {
int h = catalog->add(id); int32_t handle = catalog->add(id);
absl::optional< int > res = catalog->find(h); absl::optional< int32_t > res = catalog->find(handle);
ASSERT_TRUE(res); ASSERT_TRUE(res);
ASSERT_EQ(res.value(), id); ASSERT_EQ(res.value(), id);
ASSERT_TRUE(catalog->replace(-id - 1, h)); ASSERT_TRUE(catalog->replace(MAX - id, handle));
res = catalog->find(h); res = catalog->find(handle);
ASSERT_TRUE(res); ASSERT_TRUE(res);
ASSERT_EQ(-id - 1, res.value()); ASSERT_EQ(MAX - id, res.value());
int removed = -1; int32_t removed = 0;
ASSERT_TRUE(catalog->remove(h, &removed)); ASSERT_TRUE(catalog->remove(handle, &removed));
ASSERT_EQ(removed, -id - 1); ASSERT_EQ(removed, MAX - id);
ASSERT_FALSE(catalog->find(h)); ASSERT_FALSE(catalog->find(handle));
} }
}, },
&barrier, &catalog, i); &barrier, &catalog, i);
@ -113,30 +114,29 @@ TEST(Catalog, Iterator)
barrier->Block(); barrier->Block();
for(size_t i = 0; i < ITERATION_COUNT; ++i) for(size_t i = 0; i < ITERATION_COUNT; ++i)
{ {
int arr[100]; int32_t arr[100];
size_t size = 0; size_t size = 0;
for(Iterator it(*catalog); it; ++it) for(Iterator it(catalog); it; ++it)
{ {
arr[size++] = it().second; arr[size++] = it().second;
} }
for(int i = 0; i < 100; i++) for(int32_t j = 0; j < 100; j++)
{ {
// value must be valid // value must be valid
bool present = false; bool present = false;
for(int id = 0; id < static_cast< int >(THREAD_COUNT); id++) for(int32_t id = 0; id < static_cast< int32_t >(THREAD_COUNT); id++)
{ {
if(id == arr[i] || -id - 1 == arr[i]) if(id == arr[j] || MAX - id == arr[j])
{ {
present = true; present = true;
break; break;
} }
} }
ASSERT_TRUE(present);
// no duplicate should be there // no duplicate should be there
for(size_t j = i + 1; j < size; j++) for(size_t k = j + 1; k < size; k++)
{ {
ASSERT_NE(arr[i], arr[j]); ASSERT_NE(arr[j], arr[k]);
} }
} }
} }