Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions score/memory/shared/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ cc_library(
"@score_baselibs//score/memory/shared/fake:__subpackages__",
],
deps = [
":atomic_indirector",
":i_shared_memory_resource",
":lock_file",
":pointer_arithmetic_util",
Expand Down Expand Up @@ -786,6 +787,7 @@ cc_gtest_unit_test(
"@score_baselibs//score/memory:__pkg__",
],
deps = [
":atomic_indirector_mock_binding",
":pointer_arithmetic_util",
":shared_memory_test_resources",
"@score_baselibs//score/memory/shared/fake:fake_memory_resources",
Expand Down
109 changes: 80 additions & 29 deletions score/memory/shared/shared_memory_resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
********************************************************************************/
#include "score/memory/shared/shared_memory_resource.h"
#include "score/language/safecpp/string_view/zstring_view.h"
#include "score/memory/shared/atomic_indirector.h"
#include "score/memory/shared/memory_resource_proxy.h"
#include "score/memory/shared/memory_resource_registry.h"
#include "score/memory/shared/pointer_arithmetic_util.h"
Expand Down Expand Up @@ -688,51 +689,101 @@ auto SharedMemoryResource::getMemoryResourceProxy() noexcept -> const MemoryReso

auto SharedMemoryResource::do_allocate(const std::size_t bytes, const std::size_t alignment) -> void*
{
std::lock_guard<score::os::InterprocessMutex> lock(this->control_block_->mutex);
return do_allocate_with_indirector<AtomicIndirectorReal>(bytes, alignment);
}

template <template <class> class AtomicIndirectorType>
auto SharedMemoryResource::do_allocate_with_indirector(const std::size_t bytes, const std::size_t alignment) -> void*
{
/**
* For the proof of concept we agreed to use a monotonic allocation algorithm.
* So there is no need for any fancy logic.
*
* The bump pointer (alreadyAllocatedBytes) is advanced with a lock-free compare-exchange
* loop instead of a process-shared mutex. This keeps concurrent allocations correct even
* when several processes -- or several VMs sharing a single physical region such as a QEMU
* ivshmem PCI BAR -- allocate at the same time, and it removes the process-shared pthread
* mutex, which cannot be locked on device memory and cannot synchronize separate kernels.
*/
const std::size_t already_allocated_bytes = this->control_block_->alreadyAllocatedBytes.load();
void* const allocation_start_address =
// In our architecture we have a one-to-one mapping between pointers and integral values.
// Therefore, casting between the two is well-defined.
// The base_adress_ points to the start of the memory arena for the allocation and alreadyAllocatedBytes is
// ensured by this class to not exceed the size of the memory arena.
// Therefore, the resulting pointer will always point into the memory arena.
// In C++23 std::start_lifetime_as_array can be used to inform the compiler.
// NOLINTNEXTLINE(score-banned-function) see above
AddOffsetToPointer(this->base_address_, already_allocated_bytes);

constexpr std::size_t max_retries = 32U;

// In our architecture we have a one-to-one mapping between pointers and integral values.
// Therefore, casting between the two is well-defined.
// This pointer is not dereferenced.
// NOLINTNEXTLINE(score-banned-function) see above
void* const allocation_end_address = AddOffsetToPointer(this->base_address_, virtual_address_space_to_reserve_);
void* const new_address_aligned =
detail::do_allocation_algorithm(allocation_start_address, allocation_end_address, bytes, alignment);

if (new_address_aligned == nullptr)
std::size_t already_allocated_bytes =
AtomicIndirectorType<std::size_t>::load(this->control_block_->alreadyAllocatedBytes, std::memory_order_acquire);

for (std::size_t retry_count = 0U; retry_count < max_retries; ++retry_count)
{
score::mw::log::LogFatal("shm")
<< "Cannot allocate shared memory block of size" << bytes << "with alignment " << alignment
<< " at: ["
// In our architecture we have a one-to-one mapping between pointers and integral values.
// Therefore, casting between the two is well-defined.
// The resulting pointer is used for logging and is not dereferenced.
void* const allocation_start_address =
// The base_address_ points to the start of the memory arena for the allocation and
// alreadyAllocatedBytes is ensured by this class to not exceed the size of the memory arena.
// Therefore, the resulting pointer will always point into the memory arena.
// NOLINTNEXTLINE(score-banned-function) see above
<< PointerToLogValue(allocation_start_address) << ":" << PointerToLogValue(allocation_end_address)
<< "]. Does not fit within shared memory segment: [" << PointerToLogValue(this->base_address_) << ":"
<< PointerToLogValue(this->getEndAddress()) << "]. Already allocated bytes: " << already_allocated_bytes
<< ". Virtual address space to reserve: " << virtual_address_space_to_reserve_;
std::terminate();
AddOffsetToPointer(this->base_address_, already_allocated_bytes);

void* const new_address_aligned =
detail::do_allocation_algorithm(allocation_start_address,
allocation_end_address,
bytes,
alignment);

if (new_address_aligned == nullptr)
{
score::mw::log::LogFatal("shm")
<< "Cannot allocate shared memory block of size " << bytes
<< " with alignment " << alignment
<< " at: ["
// The resulting pointer is used for logging and is not dereferenced.
// NOLINTNEXTLINE(score-banned-function) see above
<< PointerToLogValue(allocation_start_address) << ":"
<< PointerToLogValue(allocation_end_address)
<< "]. Does not fit within shared memory segment: ["
<< PointerToLogValue(this->base_address_) << ":"
<< PointerToLogValue(this->getEndAddress())
<< "]. Already allocated bytes: " << already_allocated_bytes
<< ". Virtual address space to reserve: "
<< virtual_address_space_to_reserve_;
std::terminate();
}

const auto padding =
SubtractPointersBytes(new_address_aligned, allocation_start_address);

const auto total_allocated_bytes =
safe_math::Add(bytes, padding).value();

const std::size_t new_already_allocated_bytes =
safe_math::Add(already_allocated_bytes, total_allocated_bytes).value();

// Atomically publish the advanced bump pointer. If another allocator updated the
// bump pointer first, compare_exchange_weak updates already_allocated_bytes with the
// latest value and the next loop iteration recalculates the allocation.
if (AtomicIndirectorType<std::size_t>::compare_exchange_weak(
this->control_block_->alreadyAllocatedBytes,
already_allocated_bytes,
new_already_allocated_bytes,
std::memory_order_acq_rel,
std::memory_order_acquire))
{
return new_address_aligned;
}
}
const auto padding = SubtractPointersBytes(new_address_aligned, allocation_start_address);

const auto total_allocated_bytes = safe_math::Add(bytes, padding).value();
this->control_block_->alreadyAllocatedBytes += total_allocated_bytes;
return new_address_aligned;
score::mw::log::LogFatal("shm")
<< "Failed to reserve shared memory after " << max_retries << " attempts.";
std::terminate();
}

template void* SharedMemoryResource::do_allocate_with_indirector<AtomicIndirectorReal>(const std::size_t,
const std::size_t);
template void* SharedMemoryResource::do_allocate_with_indirector<AtomicIndirectorMock>(const std::size_t,
const std::size_t);

auto SharedMemoryResource::getBaseAddress() const noexcept -> void*
{
// Suppress "AUTOSAR C++14 A9-3-1" rule finding: "Member functions shall not return non-const “raw” pointers or
Expand Down
6 changes: 3 additions & 3 deletions score/memory/shared/shared_memory_resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ class SharedMemoryResource : public ISharedMemoryResource, public std::enable_sh
class ControlBlock
{
public:
explicit ControlBlock(const std::size_t id) noexcept : mutex{}, alreadyAllocatedBytes{}, memoryResourceProxy{id}
explicit ControlBlock(const std::size_t id) noexcept : alreadyAllocatedBytes{}, memoryResourceProxy{id}
{
}

Expand All @@ -386,8 +386,6 @@ class SharedMemoryResource : public ISharedMemoryResource, public std::enable_sh
// Rationale: There are no class invariants to maintain which could be violated by directly accessing these
// member variables.
// coverity[autosar_cpp14_m11_0_1_violation]
score::os::InterprocessMutex mutex;
// coverity[autosar_cpp14_m11_0_1_violation]
std::atomic<std::size_t> alreadyAllocatedBytes;
// coverity[autosar_cpp14_m11_0_1_violation]
MemoryResourceProxy memoryResourceProxy;
Expand Down Expand Up @@ -416,6 +414,8 @@ class SharedMemoryResource : public ISharedMemoryResource, public std::enable_sh
void* start_;

void* do_allocate(const std::size_t bytes, const std::size_t alignment) override;
template <template <class> class AtomicIndirectorType>
void* do_allocate_with_indirector(const std::size_t bytes, const std::size_t alignment);
void do_deallocate(void*, std::size_t bytes, std::size_t alignment) override;
// coverity[autosar_cpp14_m7_3_1_violation] false-positive: class method (Ticket-234468)
bool do_is_equal(const memory_resource& other) const noexcept override;
Expand Down
45 changes: 45 additions & 0 deletions score/memory/shared/shared_memory_resource_allocate_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
********************************************************************************/
#include "score/memory/shared/shared_memory_test_resources.h"

#include "score/memory/shared/atomic_mock.h"
#include "fake/my_memory_resource.h"
#include "score/memory/shared/pointer_arithmetic_util.h"

Expand Down Expand Up @@ -340,4 +341,48 @@ TEST_F(SharedMemoryResourceAllocateDeathTest, AllocatingMultipleBlocksLargerThan
EXPECT_DEATH(attorney2.getMemoryResourceProxy()->allocate(remaining_memory + 1), ".*");
}

TEST_F(SharedMemoryResourceAllocateDeathTest, AllocationTerminatesAfterMaxCompareExchangeRetries)
{
InSequence sequence{};
constexpr std::int32_t file_descriptor = 5;
constexpr bool is_read_write = true;
constexpr bool is_death_test = true;

// Given an opened shared memory resource backed by a valid control block
alignas(std::max_align_t) std::array<std::uint8_t, 300U> dataRegion{};
auto id = score::cpp::hash_bytes(TestValues::sharedMemorySegmentPath, strlen(TestValues::sharedMemorySegmentPath));
auto* const control_block_addr = new (dataRegion.data()) ControlBlock(id);
control_block_addr->alreadyAllocatedBytes = sizeof(ControlBlock);

constexpr std::int32_t lock_file_descriptor = 10;
expectCreateLockFileReturns(TestValues::sharedMemorySegmentLockPath, lock_file_descriptor);

expectShmOpenReturns(TestValues::sharedMemorySegmentPath, file_descriptor, is_read_write, is_death_test);
expectFstatReturns(file_descriptor);
expectMmapReturns(dataRegion.data(), file_descriptor, is_read_write, is_death_test);

EXPECT_CALL(*unistd_mock_, close(lock_file_descriptor));
EXPECT_CALL(*unistd_mock_, unlink(StrEq(TestValues::sharedMemorySegmentLockPath)));
EXPECT_CALL(*mman_mock_, munmap(_, _));
EXPECT_CALL(*unistd_mock_, close(file_descriptor));

auto resource_result = SharedMemoryResourceTestAttorney::Open(TestValues::sharedMemorySegmentPath, is_read_write);
ASSERT_TRUE(resource_result.has_value());
auto resource = resource_result.value();

// When every compare_exchange_weak attempt is forced to fail
// Then allocation terminates after exhausting the bounded retry loop
EXPECT_DEATH(
{
AtomicMock<std::size_t> atomic_mock{};
AtomicIndirectorMock<std::size_t>::SetMockObject(&atomic_mock);
EXPECT_CALL(atomic_mock, load(_)).WillOnce(Return(sizeof(ControlBlock)));
EXPECT_CALL(atomic_mock, compare_exchange_weak(_, _, _, _)).Times(32).WillRepeatedly(Return(false));

SharedMemoryResourceTestAttorney attorney(*resource);
attorney.do_allocate_using_mock_atomic_indirector(1U, 1U);
},
".*");
}

} // namespace score::memory::shared::test
7 changes: 7 additions & 0 deletions score/memory/shared/shared_memory_test_resources.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* SPDX-License-Identifier: Apache-2.0
********************************************************************************/
#include "score/memory/shared/shared_memory_test_resources.h"
#include "score/memory/shared/atomic_indirector.h"
#include "score/memory/shared/shared_memory_factory.h"
#include "score/os/utils/acl/i_access_control_list.h"

Expand Down Expand Up @@ -103,6 +104,12 @@ SharedMemoryResourceTestAttorney::SharedMemoryResourceTestAttorney(SharedMemoryR
{
}

void* SharedMemoryResourceTestAttorney::do_allocate_using_mock_atomic_indirector(std::size_t bytes,
std::size_t alignment)
{
return resource_.do_allocate_with_indirector<AtomicIndirectorMock>(bytes, alignment);
}

score::cpp::expected<std::shared_ptr<SharedMemoryResource>, score::os::Error> SharedMemoryResourceTestAttorney::Create(
std::string input_path,
const std::size_t user_space_to_reserve,
Expand Down
2 changes: 2 additions & 0 deletions score/memory/shared/shared_memory_test_resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ class SharedMemoryResourceTestAttorney
return resource_.do_allocate(bytes, alignment);
}

void* do_allocate_using_mock_atomic_indirector(std::size_t bytes, std::size_t alignment);

static std::size_t GetNeededManagementSpace()
{
return SharedMemoryResource::GetNeededManagementSpace();
Expand Down
Loading