This will fix existing locking issues, and provide a foundation to implement more fine-grained locking mechanisms in future works. reader_writer_lock include wait_queue to prevent writer starvation. In this design, order between read(s) and write will be preserved. Co-authored-by: Jeffro <jeffro256@tutanota.com>pull/9181/head
parent
1bec71279e
commit
84844843ac
@ -0,0 +1,125 @@
|
||||
// Copyright (c) 2024, The Monero Project
|
||||
//
|
||||
// All rights reserved.
|
||||
//
|
||||
// Redistribution and use in source and binary forms, with or without modification, are
|
||||
// permitted provided that the following conditions are met:
|
||||
//
|
||||
// 1. Redistributions of source code must retain the above copyright notice, this list of
|
||||
// conditions and the following disclaimer.
|
||||
//
|
||||
// 2. Redistributions in binary form must reproduce the above copyright notice, this list
|
||||
// of conditions and the following disclaimer in the documentation and/or other
|
||||
// materials provided with the distribution.
|
||||
//
|
||||
// 3. Neither the name of the copyright holder nor the names of its contributors may be
|
||||
// used to endorse or promote products derived from this software without specific
|
||||
// prior written permission.
|
||||
//
|
||||
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
|
||||
// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
|
||||
// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
|
||||
// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
|
||||
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
|
||||
// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
|
||||
// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
|
||||
// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
|
||||
// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
|
||||
|
||||
#include "recursive_shared_mutex.h"
|
||||
|
||||
#include <cassert>
|
||||
|
||||
namespace tools
|
||||
{
|
||||
// Initialize static member shared_slot_counter
|
||||
std::atomic<recursive_shared_mutex::slot_t> recursive_shared_mutex::shared_slot_counter{0};
|
||||
|
||||
// Initialize static member tlocal_access_per_mutex
|
||||
thread_local
|
||||
std::unordered_map<recursive_shared_mutex::slot_t, recursive_shared_mutex::access_counter_t>
|
||||
recursive_shared_mutex::tlocal_access_per_mutex{};
|
||||
|
||||
|
||||
recursive_shared_mutex::recursive_shared_mutex(): m_rw_mutex{}, m_slot{shared_slot_counter++}
|
||||
{}
|
||||
|
||||
void recursive_shared_mutex::lock()
|
||||
{
|
||||
access_counter_t &access = tlocal_access_per_mutex[m_slot];
|
||||
assert(0 == access || access & write_bit); // cannot upgrade from read->write
|
||||
|
||||
if (!access)
|
||||
m_rw_mutex.lock();
|
||||
|
||||
access = (access + 1) | write_bit;
|
||||
}
|
||||
|
||||
bool recursive_shared_mutex::try_lock()
|
||||
{
|
||||
access_counter_t &access = tlocal_access_per_mutex[m_slot];
|
||||
assert(0 == access || access & write_bit); // cannot upgrade from read->write
|
||||
|
||||
if (access || m_rw_mutex.try_lock())
|
||||
{
|
||||
access = (access + 1) | write_bit;
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!access)
|
||||
tlocal_access_per_mutex.erase(m_slot);
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
void recursive_shared_mutex::unlock()
|
||||
{
|
||||
access_counter_t &access = tlocal_access_per_mutex[m_slot];
|
||||
assert(access & depth_mask); // write depth must be non zero
|
||||
|
||||
const bool still_held = --access & depth_mask;
|
||||
if (!still_held)
|
||||
{
|
||||
m_rw_mutex.unlock();
|
||||
tlocal_access_per_mutex.erase(m_slot);
|
||||
}
|
||||
}
|
||||
|
||||
void recursive_shared_mutex::lock_shared()
|
||||
{
|
||||
access_counter_t &access = tlocal_access_per_mutex[m_slot];
|
||||
|
||||
if (!(access++))
|
||||
m_rw_mutex.lock_shared();
|
||||
}
|
||||
|
||||
bool recursive_shared_mutex::try_lock_shared()
|
||||
{
|
||||
access_counter_t &access = tlocal_access_per_mutex[m_slot];
|
||||
|
||||
if (access || m_rw_mutex.try_lock_shared())
|
||||
{
|
||||
++access;
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!access)
|
||||
tlocal_access_per_mutex.erase(m_slot);
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
void recursive_shared_mutex::unlock_shared()
|
||||
{
|
||||
access_counter_t &access = tlocal_access_per_mutex[m_slot];
|
||||
assert(access & depth_mask); // read depth must be non zero
|
||||
|
||||
const bool still_held = --access & depth_mask;
|
||||
if (!still_held)
|
||||
{
|
||||
m_rw_mutex.unlock_shared();
|
||||
tlocal_access_per_mutex.erase(m_slot);
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace tools
|
@ -0,0 +1,86 @@
|
||||
// Copyright (c) 2024, The Monero Project
|
||||
//
|
||||
// All rights reserved.
|
||||
//
|
||||
// Redistribution and use in source and binary forms, with or without modification, are
|
||||
// permitted provided that the following conditions are met:
|
||||
//
|
||||
// 1. Redistributions of source code must retain the above copyright notice, this list of
|
||||
// conditions and the following disclaimer.
|
||||
//
|
||||
// 2. Redistributions in binary form must reproduce the above copyright notice, this list
|
||||
// of conditions and the following disclaimer in the documentation and/or other
|
||||
// materials provided with the distribution.
|
||||
//
|
||||
// 3. Neither the name of the copyright holder nor the names of its contributors may be
|
||||
// used to endorse or promote products derived from this software without specific
|
||||
// prior written permission.
|
||||
//
|
||||
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
|
||||
// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
|
||||
// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
|
||||
// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
|
||||
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
|
||||
// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
|
||||
// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
|
||||
// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
|
||||
// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
|
||||
|
||||
|
||||
#pragma once
|
||||
|
||||
#include <atomic>
|
||||
#include <cstdint>
|
||||
#include <limits>
|
||||
#include <unordered_map>
|
||||
|
||||
#include <boost/thread/shared_mutex.hpp>
|
||||
|
||||
namespace tools
|
||||
{
|
||||
// Implements Lockable & SharedLockable C++14 concepts. Allows for recursion of all calls when the
|
||||
// first acquisition was exclusive, and recursion of shared access calls whens first acquisition was
|
||||
// shared. Attempting to acquire exclusive ownership of the mutex when this thread already has
|
||||
// shared ownership is undefined behavior. Maximum recursion depth is 2^31. Maxium number of
|
||||
// instances per program is 2^32. This implementation only requires one boost::shared_mutex and a
|
||||
// per-thread map of access counters, and so will be relatively performant for most use cases.
|
||||
// Because a boost::shared_mutex is used internally, access to this lock is guaranteed to be fair.
|
||||
class recursive_shared_mutex
|
||||
{
|
||||
public:
|
||||
recursive_shared_mutex();
|
||||
|
||||
// delete copy/move construct/assign operations
|
||||
recursive_shared_mutex(const recursive_shared_mutex&) = delete;
|
||||
recursive_shared_mutex(recursive_shared_mutex&&) = delete;
|
||||
recursive_shared_mutex& operator=(const recursive_shared_mutex&) = delete;
|
||||
recursive_shared_mutex& operator=(recursive_shared_mutex&&) = delete;
|
||||
|
||||
// Lockable
|
||||
void lock();
|
||||
bool try_lock();
|
||||
void unlock();
|
||||
|
||||
// SharedLockable
|
||||
void lock_shared();
|
||||
bool try_lock_shared();
|
||||
void unlock_shared();
|
||||
|
||||
private:
|
||||
using slot_t = std::uint32_t;
|
||||
using access_counter_t = std::uint32_t;
|
||||
|
||||
static constexpr access_counter_t access_counter_max = std::numeric_limits<access_counter_t>::max();
|
||||
static constexpr access_counter_t depth_mask = access_counter_max >> 1;
|
||||
static constexpr access_counter_t write_bit = access_counter_max - depth_mask;
|
||||
|
||||
// what the slot id will be for the next instance of recursive_shared_mutex
|
||||
static std::atomic<slot_t> shared_slot_counter;
|
||||
|
||||
// keeps track of read/write depth and write mode per instance of recursive_shared_mutex
|
||||
static thread_local std::unordered_map<slot_t, access_counter_t> tlocal_access_per_mutex;
|
||||
|
||||
boost::shared_mutex m_rw_mutex; // we use boost::shared_mutex since it is guaranteed fair, unlike std::shared_mutex
|
||||
const slot_t m_slot; // this is an ID number used per-thread to identify whether already held (enables recursion)
|
||||
};
|
||||
} // namespace tools
|
@ -0,0 +1,72 @@
|
||||
# Copyright (c) 2014-2023, The Monero Project
|
||||
#
|
||||
# All rights reserved.
|
||||
#
|
||||
# Redistribution and use in source and binary forms, with or without modification, are
|
||||
# permitted provided that the following conditions are met:
|
||||
#
|
||||
# 1. Redistributions of source code must retain the above copyright notice, this list of
|
||||
# conditions and the following disclaimer.
|
||||
#
|
||||
# 2. Redistributions in binary form must reproduce the above copyright notice, this list
|
||||
# of conditions and the following disclaimer in the documentation and/or other
|
||||
# materials provided with the distribution.
|
||||
#
|
||||
# 3. Neither the name of the copyright holder nor the names of its contributors may be
|
||||
# used to endorse or promote products derived from this software without specific
|
||||
# prior written permission.
|
||||
#
|
||||
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
|
||||
# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
|
||||
# MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
|
||||
# THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
|
||||
# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
|
||||
# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
|
||||
# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
|
||||
# STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
|
||||
# THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
|
||||
|
||||
set(lock_tests_source
|
||||
lock_tests.cpp)
|
||||
|
||||
monero_add_minimal_executable(lock_tests
|
||||
${lock_tests_source})
|
||||
target_link_libraries(lock_tests
|
||||
PRIVATE
|
||||
epee
|
||||
common
|
||||
${GTEST_LIBRARIES}
|
||||
${Boost_CHRONO_LIBRARY}
|
||||
${Boost_DATE_TIME_LIBRARY}
|
||||
${Boost_SYSTEM_LIBRARY}
|
||||
${Boost_THREAD_LIBARRY}
|
||||
${CMAKE_THREAD_LIBS_INIT}
|
||||
${EXTRA_LIBRARIES})
|
||||
|
||||
if(CMAKE_CXX_COMPILER_ID MATCHES GNU OR CMAKE_CXX_COMPILER_ID MATCHES Clang)
|
||||
# Due to a bug in upstream ubuntu-20.04
|
||||
# we have to disable thread sanitizer until the fix
|
||||
# pushed. Once fixed we can remove this condition and
|
||||
# set_target_properties unconditionaly.
|
||||
# https://bugs.launchpad.net/ubuntu/+source/gcc-9/+bug/2029910
|
||||
if(EXISTS "/usr/lib/x86_64-linux-gnu/libtsan_preinit.o")
|
||||
set_target_properties(lock_tests
|
||||
PROPERTIES
|
||||
COMPILE_FLAGS "-fsanitize=thread -Og -fno-omit-frame-pointer -g"
|
||||
LINK_FLAGS "-fsanitize=thread -Og -fno-omit-frame-pointer -g")
|
||||
endif()
|
||||
endif()
|
||||
|
||||
set_property(TARGET lock_tests
|
||||
PROPERTY
|
||||
FOLDER "tests")
|
||||
|
||||
add_test(
|
||||
NAME lock_tests
|
||||
COMMAND lock_tests)
|
||||
|
||||
# This is not computation intensive test.
|
||||
# just locking and unlocking in multiple threads.
|
||||
# if it fails to complete in 1500 seconds in slow machines.
|
||||
# Then it means it in somekind of deadlock, and should fail.
|
||||
set_tests_properties(lock_tests PROPERTIES TIMEOUT 1500)
|
@ -0,0 +1,185 @@
|
||||
// Copyright (c) 2018-2023, The Monero Project
|
||||
|
||||
//
|
||||
// All rights reserved.
|
||||
//
|
||||
// Redistribution and use in source and binary forms, with or without modification, are
|
||||
// permitted provided that the following conditions are met:
|
||||
//
|
||||
// 1. Redistributions of source code must retain the above copyright notice, this list of
|
||||
// conditions and the following disclaimer.
|
||||
//
|
||||
// 2. Redistributions in binary form must reproduce the above copyright notice, this list
|
||||
// of conditions and the following disclaimer in the documentation and/or other
|
||||
// materials provided with the distribution.
|
||||
//
|
||||
// 3. Neither the name of the copyright holder nor the names of its contributors may be
|
||||
// used to endorse or promote products derived from this software without specific
|
||||
// prior written permission.
|
||||
//
|
||||
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
|
||||
// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
|
||||
// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
|
||||
// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
|
||||
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
|
||||
// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
|
||||
// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
|
||||
// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
|
||||
// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
|
||||
|
||||
#include "syncobj.h"
|
||||
#include "common/recursive_shared_mutex.h"
|
||||
|
||||
#include <gtest/gtest.h>
|
||||
#include <boost/thread.hpp>
|
||||
#include <random>
|
||||
|
||||
// number of times each test runs
|
||||
int test_iteration = 2;
|
||||
|
||||
// total number of threads
|
||||
size_t total_number_of_threads;
|
||||
// Between 20-80% of the total number of threads.
|
||||
size_t number_of_writers;
|
||||
// Remaining number of threads.
|
||||
size_t number_of_readers;
|
||||
|
||||
// Cycles that readers are going to hold the lock. between 2-100 cycles
|
||||
constexpr size_t reading_cycles_min = 2;
|
||||
constexpr size_t reading_cycles_max = 25;
|
||||
// Duration that readers are going to hold the lock. between 5-100 milliseconds
|
||||
constexpr size_t reading_step_duration_min = 2;
|
||||
constexpr size_t reading_step_duration_max = 10;
|
||||
|
||||
// Cycles that writers are going to hold the lock, between 2-100 cycles
|
||||
constexpr size_t writing_cycles_min = 2;
|
||||
constexpr size_t writing_cycles_max = 25;
|
||||
// Duration that writers are going to hold the lock. between 5-100 milliseconds
|
||||
constexpr size_t writing_step_duration_min = 2;
|
||||
constexpr size_t writing_step_duration_max = 10;
|
||||
|
||||
tools::recursive_shared_mutex main_lock;
|
||||
|
||||
void calculate_parameters(size_t threads) {
|
||||
total_number_of_threads = threads;
|
||||
std::random_device dev;
|
||||
std::mt19937 rng(dev());
|
||||
std::uniform_int_distribution<std::mt19937::result_type> d( total_number_of_threads * 0.2 , total_number_of_threads * 0.8);
|
||||
|
||||
number_of_writers = d(rng);
|
||||
number_of_readers = total_number_of_threads - number_of_writers;
|
||||
}
|
||||
|
||||
void reader() {
|
||||
std::random_device dev;
|
||||
std::mt19937 rng(dev());
|
||||
std::uniform_int_distribution<std::mt19937::result_type> d(reading_cycles_min, reading_cycles_max);
|
||||
size_t reading_cycles = d(rng);
|
||||
|
||||
d = std::uniform_int_distribution<std::mt19937::result_type>(reading_step_duration_min, reading_step_duration_max);
|
||||
main_lock.lock_shared();
|
||||
for(int i = 0; i < reading_cycles; i++) {
|
||||
boost::this_thread::sleep_for(boost::chrono::milliseconds(d(rng)));
|
||||
}
|
||||
bool recurse = !((bool) std::uniform_int_distribution<std::mt19937::result_type>(0, 10)(rng) % 4); // ~30%
|
||||
if (recurse) {
|
||||
reader();
|
||||
}
|
||||
main_lock.unlock_shared();
|
||||
}
|
||||
|
||||
void writer() {
|
||||
std::random_device dev;
|
||||
std::mt19937 rng(dev());
|
||||
std::uniform_int_distribution<std::mt19937::result_type> d(writing_cycles_min, writing_cycles_max);
|
||||
size_t writing_cycles = d(rng);
|
||||
|
||||
d = std::uniform_int_distribution<std::mt19937::result_type>(writing_step_duration_min, writing_step_duration_max);
|
||||
main_lock.lock();
|
||||
for(int i = 0; i < writing_cycles; i++) {
|
||||
boost::this_thread::sleep_for(boost::chrono::milliseconds(d(rng)));
|
||||
}
|
||||
bool recurse = !((bool) std::uniform_int_distribution<std::mt19937::result_type>(0, 10)(rng) % 4); // ~20%
|
||||
if (recurse) {
|
||||
bool which = std::uniform_int_distribution<std::mt19937::result_type>(0, 10)(rng) % 2;
|
||||
if (which) {
|
||||
writer();
|
||||
}
|
||||
else {
|
||||
reader();
|
||||
}
|
||||
}
|
||||
main_lock.unlock();
|
||||
}
|
||||
|
||||
void RUN_TEST() {
|
||||
std::vector<boost::thread> threads;
|
||||
std::random_device dev;
|
||||
std::mt19937 rng(dev());
|
||||
std::uniform_int_distribution<std::mt19937::result_type> d(0, 10);
|
||||
|
||||
int reader_count = 0;
|
||||
int writer_count = 0;
|
||||
while(reader_count < number_of_readers
|
||||
|| writer_count < number_of_writers ) {
|
||||
bool which_one = d(rng) % 2;
|
||||
if(which_one) {
|
||||
threads.push_back(boost::thread(reader)); reader_count++;
|
||||
}
|
||||
else {
|
||||
threads.push_back(boost::thread(writer)); writer_count++;
|
||||
}
|
||||
}
|
||||
|
||||
std::for_each(threads.begin(), threads.end(), [] (boost::thread& thread) {
|
||||
if (thread.joinable()) thread.join();
|
||||
});
|
||||
}
|
||||
|
||||
TEST(reader_writer_lock_deadlock_tests, test_3)
|
||||
{
|
||||
calculate_parameters(3);
|
||||
for(int i = 0; i < test_iteration; ++i)
|
||||
RUN_TEST();
|
||||
}
|
||||
|
||||
TEST(reader_writer_lock_deadlock_tests, test_5)
|
||||
{
|
||||
calculate_parameters(5);
|
||||
for(int i = 0; i < test_iteration; ++i)
|
||||
RUN_TEST();
|
||||
}
|
||||
|
||||
TEST(reader_writer_lock_deadlock_tests, test_10)
|
||||
{
|
||||
calculate_parameters(10);
|
||||
for(int i = 0; i < test_iteration; ++i)
|
||||
RUN_TEST();
|
||||
}
|
||||
|
||||
TEST(reader_writer_lock_deadlock_tests, test_100)
|
||||
{
|
||||
calculate_parameters(100);
|
||||
for(int i = 0; i < test_iteration; ++i)
|
||||
RUN_TEST();
|
||||
}
|
||||
|
||||
TEST(reader_writer_lock_deadlock_tests, test_500)
|
||||
{
|
||||
calculate_parameters(500);
|
||||
for(int i = 0; i < test_iteration; ++i)
|
||||
RUN_TEST();
|
||||
}
|
||||
|
||||
TEST(reader_writer_lock_deadlock_tests, test_1000)
|
||||
{
|
||||
calculate_parameters(1000);
|
||||
for(int i = 0; i < test_iteration; ++i)
|
||||
RUN_TEST();
|
||||
}
|
||||
|
||||
|
||||
int main(int argc, char **argv) {
|
||||
testing::InitGoogleTest(&argc, argv);
|
||||
return RUN_ALL_TESTS();
|
||||
}
|
Loading…
Reference in new issue