Skip to content

Commit

Permalink
[unix] Improve naming based on suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@Sploder12
Sploder12 committed Jan 14, 2025
1 parent e54f9aa commit 0055805
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 43 deletions.
14 changes: 7 additions & 7 deletions include/multipass/platform_unix.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,23 @@

#include <vector>

#include <signal.h>
#include <csignal>

#include "singleton.h"

#define MP_POSIX_SIGNAL multipass::platform::SignalWrapper::instance()
#define MP_POSIX_SIGNAL multipass::platform::PosixSignal::instance()

namespace multipass::platform
{

class SignalWrapper : public Singleton<SignalWrapper>
class PosixSignal : public Singleton<PosixSignal>
{
public:
SignalWrapper(const PrivatePass&) noexcept;
PosixSignal(const PrivatePass&) noexcept;

virtual int mask_signals(int how, const sigset_t* sigset, sigset_t* old_set = nullptr) const;
virtual int send(pthread_t target, int signal) const;
virtual int wait(const sigset_t& sigset, int& got) const;
virtual int pthread_sigmask(int how, const sigset_t* sigset, sigset_t* old_set = nullptr) const;
virtual int pthread_kill(pthread_t target, int signal) const;
virtual int sigwait(const sigset_t& sigset, int& got) const;
};

sigset_t make_sigset(const std::vector<int>& sigs);
Expand Down
21 changes: 11 additions & 10 deletions src/platform/platform_unix.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,23 +158,23 @@ int mp::platform::symlink_attr_from(const char* path, sftp_attributes_struct* at
return 0;
}

mp::platform::SignalWrapper::SignalWrapper(const PrivatePass& pass) noexcept : Singleton(pass)
mp::platform::PosixSignal::PosixSignal(const PrivatePass& pass) noexcept : Singleton(pass)
{
}

int mp::platform::SignalWrapper::mask_signals(int how, const sigset_t* sigset, sigset_t* old_set) const
int mp::platform::PosixSignal::pthread_sigmask(int how, const sigset_t* sigset, sigset_t* old_set) const

Check warning on line 165 in src/platform/platform_unix.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/platform_unix.cpp#L165 

Added line #L165 was not covered by tests
{
return pthread_sigmask(how, sigset, old_set);
return ::pthread_sigmask(how, sigset, old_set);

Check warning on line 167 in src/platform/platform_unix.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/platform_unix.cpp#L167 

Added line #L167 was not covered by tests
}

int mp::platform::SignalWrapper::send(pthread_t target, int signal) const
int mp::platform::PosixSignal::pthread_kill(pthread_t target, int signal) const

Check warning on line 170 in src/platform/platform_unix.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/platform_unix.cpp#L170 

Added line #L170 was not covered by tests
{
return pthread_kill(target, signal);
return ::pthread_kill(target, signal);

Check warning on line 172 in src/platform/platform_unix.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/platform_unix.cpp#L172 

Added line #L172 was not covered by tests
}

int mp::platform::SignalWrapper::wait(const sigset_t& sigset, int& got) const
int mp::platform::PosixSignal::sigwait(const sigset_t& sigset, int& got) const

Check warning on line 175 in src/platform/platform_unix.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/platform_unix.cpp#L175 

Added line #L175 was not covered by tests
{
return sigwait(std::addressof(sigset), std::addressof(got));
return ::sigwait(std::addressof(sigset), std::addressof(got));

Check warning on line 177 in src/platform/platform_unix.cpp

View check run for this annotation

Codecov / codecov/patch

src/platform/platform_unix.cpp#L177 

Added line #L177 was not covered by tests
}

sigset_t mp::platform::make_sigset(const std::vector<int>& sigs)
Expand All @@ -191,7 +191,7 @@ sigset_t mp::platform::make_sigset(const std::vector<int>& sigs)
sigset_t mp::platform::make_and_block_signals(const std::vector<int>& sigs)
{
auto sigset{make_sigset(sigs)};
MP_POSIX_SIGNAL.mask_signals(SIG_BLOCK, &sigset, nullptr);
MP_POSIX_SIGNAL.pthread_sigmask(SIG_BLOCK, &sigset, nullptr);
return sigset;
}

Expand All @@ -208,7 +208,8 @@ std::function<std::optional<int>(const std::function<bool()>&)> mp::platform::ma
return [sigset = make_and_block_signals({SIGQUIT, SIGTERM, SIGHUP, SIGUSR2}),
period](const std::function<bool()>& condition) -> std::optional<int> {
// create a timer to periodically send SIGUSR2
utils::Timer signal_generator{period, [signalee = pthread_self()] { MP_POSIX_SIGNAL.send(signalee, SIGUSR2); }};
utils::Timer signal_generator{period,
[signalee = pthread_self()] { MP_POSIX_SIGNAL.pthread_kill(signalee, SIGUSR2); }};

// wait on signals and condition
int latest_signal = SIGUSR2;
Expand All @@ -217,7 +218,7 @@ std::function<std::optional<int>(const std::function<bool()>&)> mp::platform::ma
signal_generator.start();

// can't use sigtimedwait since macOS doesn't support it
MP_POSIX_SIGNAL.wait(sigset, latest_signal);
MP_POSIX_SIGNAL.sigwait(sigset, latest_signal);
}

signal_generator.stop();
Expand Down
12 changes: 6 additions & 6 deletions tests/unix/mock_signal_wrapper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@
namespace multipass::test
{

class MockSignalWrapper : public platform::SignalWrapper
class MockPosixSignal : public platform::PosixSignal
{
public:
using SignalWrapper::SignalWrapper;
using PosixSignal::PosixSignal;

MOCK_METHOD(int, mask_signals, (int, const sigset_t*, sigset_t*), (const, override));
MOCK_METHOD(int, send, (pthread_t, int), (const, override));
MOCK_METHOD(int, wait, (const sigset_t&, int&), (const, override));
MOCK_METHOD(int, pthread_sigmask, (int, const sigset_t*, sigset_t*), (const, override));
MOCK_METHOD(int, pthread_kill, (pthread_t, int), (const, override));
MOCK_METHOD(int, sigwait, (const sigset_t&, int&), (const, override));

MP_MOCK_SINGLETON_BOILERPLATE(MockSignalWrapper, platform::SignalWrapper);
MP_MOCK_SINGLETON_BOILERPLATE(MockPosixSignal, platform::PosixSignal);
};

} // namespace multipass::test
Expand Down
40 changes: 20 additions & 20 deletions tests/unix/test_platform_unix.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,11 @@ TEST_F(TestPlatformUnix, make_sigset_makes_sigset)

TEST_F(TestPlatformUnix, make_and_block_signals_works)
{
auto [mock_signals, guard] = mpt::MockSignalWrapper::inject<StrictMock>();
auto [mock_signals, guard] = mpt::MockPosixSignal::inject<StrictMock>();

EXPECT_CALL(
*mock_signals,
mask_signals(SIG_BLOCK, Pointee(Truly([](const auto& set) { return test_sigset_has(set, {SIGINT}); })), _));
pthread_sigmask(SIG_BLOCK, Pointee(Truly([](const auto& set) { return test_sigset_has(set, {SIGINT}); })), _));

auto set = mp::platform::make_and_block_signals({SIGINT});

Expand All @@ -215,53 +215,53 @@ TEST_F(TestPlatformUnix, make_and_block_signals_works)

TEST_F(TestPlatformUnix, make_quit_watchdog_blocks_signals)
{
auto [mock_signals, guard] = mpt::MockSignalWrapper::inject<StrictMock>();
auto [mock_signals, guard] = mpt::MockPosixSignal::inject<StrictMock>();

EXPECT_CALL(*mock_signals,
mask_signals(SIG_BLOCK,
Pointee(Truly([](const auto& set) {
return test_sigset_has(set, {SIGQUIT, SIGTERM, SIGHUP, SIGUSR2});
})),
_));
pthread_sigmask(SIG_BLOCK,
Pointee(Truly([](const auto& set) {
return test_sigset_has(set, {SIGQUIT, SIGTERM, SIGHUP, SIGUSR2});
})),
_));

mp::platform::make_quit_watchdog(std::chrono::milliseconds{1});
}

TEST_F(TestPlatformUnix, quit_watchdog_quits_on_condition)
{
auto [mock_signals, guard] = mpt::MockSignalWrapper::inject<StrictMock>();
auto [mock_signals, guard] = mpt::MockPosixSignal::inject<StrictMock>();

EXPECT_CALL(*mock_signals, mask_signals(SIG_BLOCK, _, _));
EXPECT_CALL(*mock_signals, wait(_, _)).WillRepeatedly(DoAll(SetArgReferee<1>(SIGUSR2), Return(0)));
ON_CALL(*mock_signals, send(pthread_self(), SIGUSR2)).WillByDefault(Return(0));
EXPECT_CALL(*mock_signals, pthread_sigmask(SIG_BLOCK, _, _));
EXPECT_CALL(*mock_signals, sigwait(_, _)).WillRepeatedly(DoAll(SetArgReferee<1>(SIGUSR2), Return(0)));
ON_CALL(*mock_signals, pthread_kill(pthread_self(), SIGUSR2)).WillByDefault(Return(0));

auto watchdog = mp::platform::make_quit_watchdog(std::chrono::milliseconds{1});
EXPECT_EQ(watchdog([] { return false; }), std::nullopt);
}

TEST_F(TestPlatformUnix, quit_watchdog_quits_on_signal)
{
auto [mock_signals, guard] = mpt::MockSignalWrapper::inject<StrictMock>();
auto [mock_signals, guard] = mpt::MockPosixSignal::inject<StrictMock>();

EXPECT_CALL(*mock_signals, mask_signals(SIG_BLOCK, _, _));
EXPECT_CALL(*mock_signals, wait(_, _))
EXPECT_CALL(*mock_signals, pthread_sigmask(SIG_BLOCK, _, _));
EXPECT_CALL(*mock_signals, sigwait(_, _))
.WillOnce(DoAll(SetArgReferee<1>(SIGUSR2), Return(0)))
.WillOnce(DoAll(SetArgReferee<1>(SIGTERM), Return(0)));
ON_CALL(*mock_signals, send(pthread_self(), SIGUSR2)).WillByDefault(Return(0));
ON_CALL(*mock_signals, pthread_kill(pthread_self(), SIGUSR2)).WillByDefault(Return(0));

auto watchdog = mp::platform::make_quit_watchdog(std::chrono::milliseconds{1});
EXPECT_EQ(watchdog([] { return true; }), SIGTERM);
}

TEST_F(TestPlatformUnix, quit_watchdog_signals_itself_asynchronously)
{
auto [mock_signals, guard] = mpt::MockSignalWrapper::inject<StrictMock>();
auto [mock_signals, guard] = mpt::MockPosixSignal::inject<StrictMock>();

std::atomic<bool> signaled = false;
std::atomic<int> times = 0;

EXPECT_CALL(*mock_signals, mask_signals(SIG_BLOCK, _, _));
EXPECT_CALL(*mock_signals, wait(_, _))
EXPECT_CALL(*mock_signals, pthread_sigmask(SIG_BLOCK, _, _));
EXPECT_CALL(*mock_signals, sigwait(_, _))
.WillRepeatedly(DoAll(
[&signaled, &times] {
// busy wait until signaled
Expand All @@ -275,7 +275,7 @@ TEST_F(TestPlatformUnix, quit_watchdog_signals_itself_asynchronously)
SetArgReferee<1>(SIGUSR2),
Return(0)));

EXPECT_CALL(*mock_signals, send(pthread_self(), SIGUSR2))
EXPECT_CALL(*mock_signals, pthread_kill(pthread_self(), SIGUSR2))
.WillRepeatedly(DoAll([&signaled] { signaled.store(true, std::memory_order_release); }, Return(0)));

auto watchdog = mp::platform::make_quit_watchdog(std::chrono::milliseconds{1});
Expand Down

0 comments on commit 0055805

Please sign in to comment.