Fix Catalog.Iterator test failure

pull/472/head
Michael 5 years ago
parent 3306f00cf1
commit 6b5a688e08
No known key found for this signature in database
GPG Key ID: 2D51757B47E2434C

@ -30,7 +30,7 @@ endif()
if(${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
# check if we have the (saner) emulation of epoll here
# it's basically linux epoll but with a sane method of
# it's basically linux epoll but with a sane method of
# dealing with closed file handles that still exist in the
# epoll set
#
@ -161,7 +161,7 @@ find_package(Threads REQUIRED)
# not supported on Solaris - system libraries are not available as archives
if(STATIC_LINK_RUNTIME)
if (NOT SOLARIS)
add_compile_options(-static)
add_compile_options(-static)
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -static -static-libstdc++ -pthread" )
else()
@ -190,7 +190,7 @@ if(NOT DEBIAN)
endif(NOT DEBIAN)
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")
endif(ASAN)

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

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

Loading…
Cancel
Save