Skip to content

Commit

Permalink
Merge pull request #2332 from elBoberido/iox-2329-use-iox-atomic
Browse files Browse the repository at this point in the history
iox-#2329 Use iox::concurrent::Atomic
  • Loading branch information
elBoberido authored Aug 22, 2024
2 parents 8a2a18f + bf4e66a commit 5bca9a4
Show file tree
Hide file tree
Showing 110 changed files with 465 additions and 393 deletions.
1 change: 1 addition & 0 deletions .cirrus.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ preflight_check_task:
clang_format_script: tools/scripts/clang_format.sh check
check_test_ids_script: tools/scripts/check_test_ids.sh
check_invalid_characters_script: tools/scripts/check_invalid_characters.sh
check_atomic_usage: tools/scripts/check_atomic_usage.sh
cmake_linter_script: tools/ci/cmake-linter.sh

#
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ jobs:
- run: ./tools/scripts/clang_format.sh check
- run: ./tools/scripts/check_test_ids.sh
- run: ./tools/scripts/check_invalid_characters.sh
- run: ./tools/scripts/check_atomic_usage.sh
- run: ./tools/ci/cmake-linter.sh

check-status-of-nightly-action:
Expand Down
38 changes: 19 additions & 19 deletions doc/design/listener.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,22 @@ which event is attached, like in the _WaitSet_.
| - m_toBeDestroyed |
| - m_activeNotifications |
+---------------------------+
| 1 | 1
| |
| 1 | n
+-----------------------------------------------+ +--------------------------------------------------+
| ConditionListener | | ConditionNotifier |
| ConditionListener(ConditionVariableData & ) | | ConditionNotifier(ConditionVariableData &, |
| | | uint64_t notificationIndex) |
| bool wasNotified() | | |
| void destroy() | | void notify() |
| NotificationVector_t wait() | | |
| NotificationVector_t timedWait() | | - m_condVarDataPtr : ConditionVariableData* |
| | | - m_notificationIndex |
| - m_condVarDataPtr : ConditionVariableData* | +--------------------------------------------------+
| - m_toBeDestroyed : std::atomic_bool | | 1
+-----------------------------------------------+ |
| 1 | n
| 1 | 1
| |
| 1 | n
+------------------------------------------------------+ +--------------------------------------------------+
| ConditionListener | | ConditionNotifier |
| ConditionListener(ConditionVariableData & ) | | ConditionNotifier(ConditionVariableData &, |
| | | uint64_t notificationIndex) |
| bool wasNotified() | | |
| void destroy() | | void notify() |
| NotificationVector_t wait() | | |
| NotificationVector_t timedWait() | | - m_condVarDataPtr : ConditionVariableData* |
| | | - m_notificationIndex |
| - m_condVarDataPtr : ConditionVariableData* | +--------------------------------------------------+
| - m_toBeDestroyed : iox::concurrent::Atomic<bool> | | 1
+------------------------------------------------------+ |
| 1 | n
| +--------------------------------+
| 1 | TriggerHandle |
+-------------------------------------------------+ | bool isValid() |
Expand Down Expand Up @@ -171,8 +171,8 @@ Triggerable TriggerHandle ConditionNotifier ConditionListener
| | | | | m_events[notificationId] |
```

- **Triggerable goes out of scope:** The TriggerHandle is a member of the
Triggerable, therefore the d'tor of the TriggerHandle is called which removes
- **Triggerable goes out of scope:** The TriggerHandle is a member of the
Triggerable, therefore the d'tor of the TriggerHandle is called which removes
the trigger from the Listener via the `resetCallback`

```
Expand All @@ -183,7 +183,7 @@ Triggerable TriggerHandle Listener Event_t
| | via resetCallback | ------------> |
```

- **Listener goes out of scope:** The d'tor of the `Event_t` invalidates the
- **Listener goes out of scope:** The d'tor of the `Event_t` invalidates the
Trigger inside the Triggerable via the `invalidationCallback`

```
Expand Down
2 changes: 1 addition & 1 deletion doc/design/relocatable_pointer.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ the operations incur slightly more overhead compared to relocatable pointers.

## Atomic usage

There is a technical problem using both `relocatable_ptr` and `RelativePointer` as a type in a `std::atomic`.
There is a technical problem using both `relocatable_ptr` and `RelativePointer` as a type in a `iox::concurrent::Atomic`.
This is essentially impossible as an atomic requires its type to be copyable/movable (to be loaded) but on the other hand,
this copy constructor must be trivial, i.e. performable with a (shallow) memcpy. Therefore, the types used in atomic cannot implement
custom copy/move. This is not possible for `relocatable_ptr` and `RelativePointer` as both require operations performed
Expand Down
1 change: 0 additions & 1 deletion iceoryx_binding_c/test/moduletests/test_listener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ extern "C" {

#include "test.hpp"

#include <atomic>
#include <thread>

namespace
Expand Down
12 changes: 6 additions & 6 deletions iceoryx_binding_c/test/moduletests/test_wait_set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "iceoryx_posh/popo/untyped_server.hpp"
#include "iceoryx_posh/popo/user_trigger.hpp"
#include "iceoryx_posh/runtime/service_discovery.hpp"
#include "iox/atomic.hpp"
#include "iox/detail/hoofs_error_reporting.hpp"

#include "iceoryx_hoofs/testing/fatal_failure.hpp"
Expand All @@ -45,7 +46,6 @@ extern "C" {

#include "test.hpp"

#include <atomic>
#include <thread>

namespace
Expand Down Expand Up @@ -986,7 +986,7 @@ TIMING_TEST_F(iox_ws_test, WaitIsBlockingTillTriggered, Repeat(5), [&] {
::testing::Test::RecordProperty("TEST_ID", "6d8a476d-5bcd-45a5-bbd4-7b3b709ac967");
iox_ws_attach_user_trigger_event(m_sut, m_userTrigger[0U], 0U, userTriggerCallback);

std::atomic_bool waitWasCalled{false};
iox::concurrent::Atomic<bool> waitWasCalled{false};
std::thread t([&] {
iox_ws_wait(m_sut, NULL, 0U, &m_missedElements);
waitWasCalled.store(true);
Expand All @@ -1003,7 +1003,7 @@ TIMING_TEST_F(iox_ws_test, WaitIsBlockingTillTriggered, Repeat(5), [&] {

TIMING_TEST_F(iox_ws_test, WaitIsNonBlockingAfterMarkForDestruction, Repeat(5), [&] {
::testing::Test::RecordProperty("TEST_ID", "4e576665-fda1-4f3c-8588-e9d2cffcb3f4");
std::atomic_bool waitWasCalled{false};
iox::concurrent::Atomic<bool> waitWasCalled{false};
std::thread t([&] {
iox_ws_wait(m_sut, NULL, 0U, &m_missedElements);
iox_ws_wait(m_sut, NULL, 0U, &m_missedElements);
Expand All @@ -1025,7 +1025,7 @@ TIMING_TEST_F(iox_ws_test, TimedWaitIsBlockingTillTriggered, Repeat(5), [&] {
::testing::Test::RecordProperty("TEST_ID", "e79edc1d-8b8a-4dd0-97ba-e2f41c9c8b31");
iox_ws_attach_user_trigger_event(m_sut, m_userTrigger[0U], 0U, userTriggerCallback);

std::atomic_bool waitWasCalled{false};
iox::concurrent::Atomic<bool> waitWasCalled{false};
std::thread t([&] {
iox_ws_timed_wait(m_sut, {1000, 1000}, NULL, 0U, &m_missedElements);
waitWasCalled.store(true);
Expand All @@ -1042,7 +1042,7 @@ TIMING_TEST_F(iox_ws_test, TimedWaitIsBlockingTillTriggered, Repeat(5), [&] {

TIMING_TEST_F(iox_ws_test, TimedWaitIsNonBlockingAfterMarkForDestruction, Repeat(5), [&] {
::testing::Test::RecordProperty("TEST_ID", "a6da4f49-b162-4c70-b0fa-c4ef1f988c57");
std::atomic_bool waitWasCalled{false};
iox::concurrent::Atomic<bool> waitWasCalled{false};
std::thread t([&] {
iox_ws_timed_wait(m_sut, {1000, 1000}, NULL, 0U, &m_missedElements);
iox_ws_timed_wait(m_sut, {1000, 1000}, NULL, 0U, &m_missedElements);
Expand All @@ -1063,7 +1063,7 @@ TIMING_TEST_F(iox_ws_test, TimedWaitBlocksTillTimeout, Repeat(5), [&] {
::testing::Test::RecordProperty("TEST_ID", "12fbbbc8-80b2-4e7e-af41-1376b2e48f4a");
iox_ws_attach_user_trigger_event(m_sut, m_userTrigger[0U], 0U, userTriggerCallback);

std::atomic_bool waitWasCalled{false};
iox::concurrent::Atomic<bool> waitWasCalled{false};
std::thread t([&] {
constexpr long hundredMsInNanoSeconds = 100000000L;
iox_ws_timed_wait(m_sut, {0, hundredMsInNanoSeconds}, NULL, 0U, &m_missedElements);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@

#include "topic_data.hpp"

#include "iox/atomic.hpp"
#include "iox/optional.hpp"
#include "iox/posh/experimental/node.hpp"
#include "iox/signal_handler.hpp"

#include <iostream>

std::atomic_bool keep_running{true};
iox::concurrent::Atomic<bool> keep_running{true};

using WaitSet = iox::popo::WaitSet<>;
volatile WaitSet* waitsetSigHandlerAccess{nullptr};
Expand Down
4 changes: 2 additions & 2 deletions iceoryx_examples/icediscovery/include/discovery_blocking.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

#include "iceoryx_posh/popo/wait_set.hpp"
#include "iceoryx_posh/runtime/service_discovery.hpp"
#include "iox/atomic.hpp"

#include <atomic>
#include <vector>

namespace discovery
Expand Down Expand Up @@ -61,7 +61,7 @@ class Discovery
private:
ServiceDiscovery* m_discovery{nullptr};
iox::popo::WaitSet<1> m_waitset;
std::atomic_bool m_blocking{true};
iox::concurrent::Atomic<bool> m_blocking{true};
};

//! [wait until condition]
Expand Down
1 change: 0 additions & 1 deletion iceoryx_examples/request_response/client_cxx_waitset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include "iox/signal_watcher.hpp"
//! [iceoryx includes]

#include <atomic>
#include <iostream>

constexpr char APP_NAME[] = "iox-cpp-request-response-client-waitset";
Expand Down
1 change: 0 additions & 1 deletion iceoryx_examples/singleprocess/single_process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#include "iox/logging.hpp"
#include "iox/signal_watcher.hpp"

#include <atomic>
#include <chrono>
#include <cstdint>
#include <iostream>
Expand Down
1 change: 0 additions & 1 deletion iceoryx_examples/user_header/publisher_cxx_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "iox/signal_watcher.hpp"
//! [iceoryx includes]

#include <atomic>
#include <iostream>

int main()
Expand Down
1 change: 0 additions & 1 deletion iceoryx_examples/user_header/publisher_untyped_cxx_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include "iceoryx_posh/runtime/posh_runtime.hpp"
//! [iceoryx includes]

#include <atomic>
#include <iostream>

int main()
Expand Down
1 change: 0 additions & 1 deletion iceoryx_examples/user_header/subscriber_cxx_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "iox/signal_watcher.hpp"
//! [iceoryx includes]

#include <atomic>
#include <iostream>

int main()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "iox/signal_watcher.hpp"
//! [iceoryx includes]

#include <atomic>
#include <iostream>

int main()
Expand Down
1 change: 0 additions & 1 deletion iceoryx_examples/waitset/ice_waitset_basic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "iox/signal_handler.hpp"
#include "topic_data.hpp"

#include <atomic>
#include <iostream>

//! [sig handler]
Expand Down
3 changes: 2 additions & 1 deletion iceoryx_examples/waitset/ice_waitset_gateway.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "iceoryx_posh/popo/user_trigger.hpp"
#include "iceoryx_posh/popo/wait_set.hpp"
#include "iceoryx_posh/runtime/posh_runtime.hpp"
#include "iox/atomic.hpp"
#include "iox/signal_handler.hpp"
#include "topic_data.hpp"

Expand All @@ -26,7 +27,7 @@

constexpr uint64_t NUMBER_OF_SUBSCRIBERS = 2U;

std::atomic_bool keepRunning{true};
iox::concurrent::Atomic<bool> keepRunning{true};

//! [waitset type alias]
using WaitSet = iox::popo::WaitSet<NUMBER_OF_SUBSCRIBERS>;
Expand Down
3 changes: 2 additions & 1 deletion iceoryx_examples/waitset/ice_waitset_grouping.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@
#include "iceoryx_posh/popo/user_trigger.hpp"
#include "iceoryx_posh/popo/wait_set.hpp"
#include "iceoryx_posh/runtime/posh_runtime.hpp"
#include "iox/atomic.hpp"
#include "iox/signal_handler.hpp"
#include "topic_data.hpp"

#include <chrono>
#include <iostream>

std::atomic_bool keepRunning{true};
iox::concurrent::Atomic<bool> keepRunning{true};

constexpr uint64_t NUMBER_OF_SUBSCRIBERS = 4U;
using WaitSet = iox::popo::WaitSet<NUMBER_OF_SUBSCRIBERS>;
Expand Down
3 changes: 2 additions & 1 deletion iceoryx_examples/waitset/ice_waitset_individual.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@
#include "iceoryx_posh/popo/user_trigger.hpp"
#include "iceoryx_posh/popo/wait_set.hpp"
#include "iceoryx_posh/runtime/posh_runtime.hpp"
#include "iox/atomic.hpp"
#include "iox/signal_handler.hpp"
#include "topic_data.hpp"

#include <chrono>
#include <iostream>

std::atomic_bool keepRunning{true};
iox::concurrent::Atomic<bool> keepRunning{true};

using WaitSet = iox::popo::WaitSet<>;
volatile WaitSet* waitsetSigHandlerAccess{nullptr};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@
#include "iceoryx_posh/popo/user_trigger.hpp"
#include "iceoryx_posh/popo/wait_set.hpp"
#include "iceoryx_posh/runtime/posh_runtime.hpp"
#include "iox/atomic.hpp"
#include "iox/signal_handler.hpp"
#include "topic_data.hpp"

#include <chrono>
#include <iostream>

std::atomic_bool keepRunning{true};
iox::concurrent::Atomic<bool> keepRunning{true};

using WaitSet = iox::popo::WaitSet<>;
volatile iox::popo::WaitSet<>* waitsetSigHandlerAccess{nullptr};
Expand Down
3 changes: 2 additions & 1 deletion iceoryx_examples/waitset/ice_waitset_trigger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@
#include "iceoryx_posh/popo/enum_trigger_type.hpp"
#include "iceoryx_posh/popo/wait_set.hpp"
#include "iceoryx_posh/runtime/posh_runtime.hpp"
#include "iox/atomic.hpp"
#include "iox/optional.hpp"

#include <iostream>
#include <thread>

std::atomic_bool keepRunning{true};
iox::concurrent::Atomic<bool> keepRunning{true};

using WaitSet = iox::popo::WaitSet<>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@
#ifndef IOX_HOOFS_CONCURRENT_BUFFER_MPMC_LOCKFREE_QUEUE_HPP
#define IOX_HOOFS_CONCURRENT_BUFFER_MPMC_LOCKFREE_QUEUE_HPP

#include "iox/atomic.hpp"
#include "iox/detail/mpmc_lockfree_queue/mpmc_index_queue.hpp"
#include "iox/optional.hpp"
#include "iox/uninitialized_array.hpp"

#include <atomic>


namespace iox
{
namespace concurrent
Expand Down Expand Up @@ -112,7 +110,7 @@ class MpmcLockFreeQueue

UninitializedArray<ElementType, Capacity> m_buffer;

std::atomic<uint64_t> m_size{0U};
Atomic<uint64_t> m_size{0U};

// template is needed to distinguish between lvalue and rvalue T references
// (universal reference type deduction)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
#ifndef IOX_HOOFS_CONCURRENT_BUFFER_MPMC_LOCKFREE_QUEUE_MPMC_INDEX_QUEUE_HPP
#define IOX_HOOFS_CONCURRENT_BUFFER_MPMC_LOCKFREE_QUEUE_MPMC_INDEX_QUEUE_HPP

#include "iox/atomic.hpp"
#include "iox/detail/mpmc_lockfree_queue/cyclic_index.hpp"
#include "iox/optional.hpp"

#include <atomic>
#include <type_traits>

namespace iox
Expand Down Expand Up @@ -109,7 +109,7 @@ class MpmcIndexQueue
// remark: a compile time check whether Index is actually lock free would be nice
// note: there is a way with is_always_lock_free in c++17 (which we cannot use here)
using Index = CyclicIndex<Capacity>;
using Cell = std::atomic<Index>;
using Cell = Atomic<Index>;

/// the array entries have to be initialized explicitly in the constructor since
/// the default atomic constructor does not call the default constructor of the
Expand All @@ -119,8 +119,8 @@ class MpmcIndexQueue
// NOLINTNEXTLINE(*avoid-c-arrays)
Cell m_cells[Capacity];

std::atomic<Index> m_readPosition;
std::atomic<Index> m_writePosition;
Atomic<Index> m_readPosition;
Atomic<Index> m_writePosition;

/// @brief load the value from m_cells at a position with a given memory order
/// @param position position to load the value from
Expand Down
Loading

0 comments on commit 5bca9a4

Please sign in to comment.