From 835a1e3c771818e6f95b16ea9432af71159dc919 Mon Sep 17 00:00:00 2001
From: rahulsutariya <Rahul.Sutariya@bti.bmwgroup.com>
Date: Thu, 9 Jul 2026 14:29:11 +0530
Subject: [PATCH] Add bounded lock-free allocation using atomic bump pointer

- Replace InterprocessMutex protection with compare_exchange_weak
- Limit CAS attempts to 32 for deterministic execution
- Handle allocation failure after retry exhaustion

Issue: https://github.com/eclipse-score/baselibs/issues/348
---
 score/memory/shared/BUILD                     |   2 +
 .../memory/shared/shared_memory_resource.cpp  | 109 +++++++++++++-----
 score/memory/shared/shared_memory_resource.h  |   6 +-
 .../shared_memory_resource_allocate_test.cpp  |  45 ++++++++
 .../shared/shared_memory_test_resources.cpp   |   7 ++
 .../shared/shared_memory_test_resources.h     |   2 +
 6 files changed, 139 insertions(+), 32 deletions(-)

diff --git a/score/memory/shared/BUILD b/score/memory/shared/BUILD
index f739bdb0d1..44acf8e38c 100644
--- a/score/memory/shared/BUILD
+++ b/score/memory/shared/BUILD
@@ -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",
@@ -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",
diff --git a/score/memory/shared/shared_memory_resource.cpp b/score/memory/shared/shared_memory_resource.cpp
index 0cc2c44477..82c2c4621e 100644
--- a/score/memory/shared/shared_memory_resource.cpp
+++ b/score/memory/shared/shared_memory_resource.cpp
@@ -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"
@@ -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
diff --git a/score/memory/shared/shared_memory_resource.h b/score/memory/shared/shared_memory_resource.h
index 23462743d5..916a6d2347 100644
--- a/score/memory/shared/shared_memory_resource.h
+++ b/score/memory/shared/shared_memory_resource.h
@@ -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}
         {
         }
 
@@ -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;
@@ -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;
diff --git a/score/memory/shared/shared_memory_resource_allocate_test.cpp b/score/memory/shared/shared_memory_resource_allocate_test.cpp
index 6e87164775..13ef0dcc59 100644
--- a/score/memory/shared/shared_memory_resource_allocate_test.cpp
+++ b/score/memory/shared/shared_memory_resource_allocate_test.cpp
@@ -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"
 
@@ -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
diff --git a/score/memory/shared/shared_memory_test_resources.cpp b/score/memory/shared/shared_memory_test_resources.cpp
index 4ea6514e59..288b8146c7 100644
--- a/score/memory/shared/shared_memory_test_resources.cpp
+++ b/score/memory/shared/shared_memory_test_resources.cpp
@@ -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"
 
@@ -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,
diff --git a/score/memory/shared/shared_memory_test_resources.h b/score/memory/shared/shared_memory_test_resources.h
index d64ee264de..0d3d0e0526 100644
--- a/score/memory/shared/shared_memory_test_resources.h
+++ b/score/memory/shared/shared_memory_test_resources.h
@@ -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();