Skip to content

Commit

Permalink
iox-eclipse-iceoryx#751 .expect prints error message with LogFatal, S…
Browse files Browse the repository at this point in the history
…ignalWatcher made signal safe and added comments to explain signal safe nature

Signed-off-by: Christian Eltzschig <christian.eltzschig@apex.ai>
  • Loading branch information
elfenpiff committed Jun 23, 2022
1 parent 1a15a54 commit 394f231
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 10 deletions.
1 change: 1 addition & 0 deletions iceoryx_hoofs/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ iox_add_library(
source/cxx/adaptive_wait.cpp
source/cxx/deadline_timer.cpp
source/cxx/filesystem.cpp
source/cxx/functional_interface.cpp
source/cxx/helplets.cpp
source/cxx/requires.cpp
source/cxx/unique_id.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ struct HasGetErrorMethod<Derived, cxx::void_t<decltype(std::declval<Derived>().g
{
};

void print_expect_message(const char* message) noexcept;

template <typename Derived>
struct Expect
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ inline void Expect<Derived>::expect(const StringType& msg) const noexcept

if (!(*static_cast<const Derived*>(this)))
{
// it is possible that expect is called inside a signal handler therefore we
// use write
auto result = write(STDERR_FILENO, &msg[0], strlen(&msg[0]));
IOX_DISCARD_RESULT(result);
print_expect_message(&msg[0]);
Ensures(false);
}
}
Expand All @@ -56,10 +53,7 @@ inline ValueType& ExpectWithValue<Derived, ValueType>::expect(const StringType&

if (!(*derivedThis))
{
// it is possible that expect is called inside a signal handler therefore we
// use write
auto result = write(STDERR_FILENO, &msg[0], strlen(&msg[0]));
IOX_DISCARD_RESULT(result);
print_expect_message(&msg[0]);
Ensures(false);
}

Expand Down
34 changes: 34 additions & 0 deletions iceoryx_hoofs/source/cxx/functional_interface.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright (c) 2022 by Apex.AI Inc. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// SPDX-License-Identifier: Apache-2.0

#include "iceoryx_hoofs/cxx/functional_interface.hpp"
#include "iceoryx_hoofs/internal/log/hoofs_logging.hpp"

namespace iox
{
namespace cxx
{
namespace internal
{
void print_expect_message(const char* message) noexcept
{
// print_expect_message is only called from expect. expect allows only
// cxx::string or char arrays which are both correctly null terminated
LogFatal() << message;
}
} // namespace internal
} // namespace cxx
} // namespace iox
15 changes: 13 additions & 2 deletions iceoryx_hoofs/source/posix_wrapper/signal_watcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// SPDX-License-Identifier: Apache-2.0
#include "iceoryx_hoofs/posix_wrapper/signal_watcher.hpp"
#include "iceoryx_hoofs/cxx/helplets.hpp"
#include "iceoryx_hoofs/internal/log/hoofs_logging.hpp"
#include "iceoryx_hoofs/platform/unistd.hpp"

namespace iox
Expand All @@ -29,7 +30,14 @@ void internalSignalHandler(int) noexcept
for (uint64_t remainingNumberOfWaiters = instance.m_numberOfWaiters.load(); remainingNumberOfWaiters > 0;
--remainingNumberOfWaiters)
{
instance.m_semaphore->post().expect("Unable to increment semaphore in signal handler");
if (instance.m_semaphore->post().has_error())
{
// we use write since internalSignalHandler can be called from within a
// signal handler and write is signal safe
constexpr const char MSG[] = "Unable to increment semaphore in signal handler";
auto result = write(STDERR_FILENO, &MSG[0], strlen(&MSG[0]));
IOX_DISCARD_RESULT(result);
}
}
}

Expand All @@ -40,6 +48,10 @@ SignalWatcher::SignalWatcher() noexcept
UnnamedSemaphoreBuilder()
.isInterProcessCapable(false)
.create(m_semaphore)

// This can be safely used despite getInstance is used in the internalSignalHandler
// since this object has to be created first before internalSignalHandler can be called.
// The only way this object can be created is by calling getInstance.
.expect("Unable to create semaphore for signal watcher");
}

Expand Down Expand Up @@ -74,6 +86,5 @@ bool hasTerminationRequested() noexcept
{
return SignalWatcher::getInstance().wasSignalTriggered();
}

} // namespace posix
} // namespace iox

0 comments on commit 394f231

Please sign in to comment.