Skip to content

Commit

Permalink
Better signals interface
Browse files Browse the repository at this point in the history
This avoids some CPP and accidentally using Unix stuff in client code.
  • Loading branch information
Ericson2314 committed Apr 4, 2024
1 parent 12ec315 commit 1a1448b
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 48 deletions.
2 changes: 1 addition & 1 deletion src/libcmd/repl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ ProcessLineResult NixRepl::processLine(std::string line)
if (line.empty())
return ProcessLineResult::PromptAgain;

_isInterrupted = false;
setInterrupted(false);

std::string command, arg;

Expand Down
4 changes: 2 additions & 2 deletions src/libfetchers/git-utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this<GitRepoImpl>
{
auto act = (Activity *) payload;
act->result(resFetchStatus, trim(std::string_view(str, len)));
return _isInterrupted ? -1 : 0;
return getInterrupted() ? -1 : 0;
}

static int transferProgressCallback(const git_indexer_progress * stats, void * payload)
Expand All @@ -361,7 +361,7 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this<GitRepoImpl>
stats->indexed_deltas,
stats->total_deltas,
stats->received_bytes / (1024.0 * 1024.0)));
return _isInterrupted ? -1 : 0;
return getInterrupted() ? -1 : 0;
}

void fetch(
Expand Down
2 changes: 1 addition & 1 deletion src/libmain/shared.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ void initNix()

initLibStore();

startSignalHandlerThread();
unix::startSignalHandlerThread();

/* Reset SIGCHLD to its default. */
struct sigaction act;
Expand Down
3 changes: 2 additions & 1 deletion src/libstore/daemon.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "daemon.hh"
#include "monitor-fd.hh"
#include "signals.hh"
#include "worker-protocol.hh"
#include "worker-protocol-impl.hh"
#include "build-result.hh"
Expand Down Expand Up @@ -1038,7 +1039,7 @@ void processConnection(
unsigned int opCount = 0;

Finally finally([&]() {
_isInterrupted = false;
setInterrupted(false);
printMsgUsing(prevLogger, lvlDebug, "%d operations", opCount);
});

Expand Down
8 changes: 4 additions & 4 deletions src/libstore/filetransfer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,11 @@ struct curlFileTransfer : public FileTransfer
int progressCallback(double dltotal, double dlnow)
{
try {
act.progress(dlnow, dltotal);
act.progress(dlnow, dltotal);
} catch (nix::Interrupted &) {
assert(_isInterrupted);
assert(getInterrupted());
}
return _isInterrupted;
return getInterrupted();
}

static int progressCallbackWrapper(void * userp, double dltotal, double dlnow, double ultotal, double ulnow)
Expand Down Expand Up @@ -466,7 +466,7 @@ struct curlFileTransfer : public FileTransfer
if (errorSink)
response = std::move(errorSink->s);
auto exc =
code == CURLE_ABORTED_BY_CALLBACK && _isInterrupted
code == CURLE_ABORTED_BY_CALLBACK && getInterrupted()
? FileTransferError(Interrupted, std::move(response), "%s of '%s' was interrupted", request.verb(), request.uri)
: httpStatus != 0
? FileTransferError(err,
Expand Down
2 changes: 1 addition & 1 deletion src/libutil/current-process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ void setStackSize(rlim_t stackSize)

void restoreProcessContext(bool restoreMounts)
{
restoreSignals();
unix::restoreSignals();
if (restoreMounts) {
#if __linux__
restoreMountNamespace();
Expand Down
70 changes: 70 additions & 0 deletions src/libutil/signals.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
#pragma once
///@file

#include "types.hh"
#include "error.hh"
#include "logging.hh"

#include <functional>

namespace nix {

/* User interruption. */

/**
* @note Does nothing on Windows
*/
static inline void setInterrupted(bool isInterrupted);

/**
* @note Does nothing on Windows
*/
static inline bool getInterrupted();

/**
* @note Does nothing on Windows
*/
static inline void setInterruptCheck(std::function<bool()> interruptCheck);

/**
* @note Does nothing on Windows
*/
void setInterruptThrown();

/**
* @note Does nothing on Windows
*/
inline void checkInterrupt();

/**
* @note Never will happen on Windows
*/
MakeError(Interrupted, BaseError);


struct InterruptCallback
{
virtual ~InterruptCallback() { };
};

/**
* Register a function that gets called on SIGINT (in a non-signal
* context).
*
* @note Does nothing on Windows
*/
std::unique_ptr<InterruptCallback> createInterruptCallback(
std::function<void()> callback);

/**
* A RAII class that causes the current thread to receive SIGUSR1 when
* the signal handler thread receives SIGINT. That is, this allows
* SIGINT to be multiplexed to multiple threads.
*
* @note Does nothing on Windows
*/
struct ReceiveInterrupts;

}

#include "signals-impl.hh"
2 changes: 1 addition & 1 deletion src/libutil/thread-pool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ void ThreadPool::doWork(bool mainThread)
ReceiveInterrupts receiveInterrupts;

if (!mainThread)
interruptCheck = [&]() { return (bool) quit; };
unix::interruptCheck = [&]() { return (bool) quit; };

bool didWork = false;
std::exception_ptr exc;
Expand Down
2 changes: 1 addition & 1 deletion src/libutil/unix/monitor-fd.hh
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public:
*/
if (count == 0) continue;
if (fds[0].revents & POLLHUP) {
triggerInterrupt();
unix::triggerInterrupt();
break;
}
/* This will only happen on macOS. We sleep a bit to
Expand Down
61 changes: 34 additions & 27 deletions src/libutil/unix/signals.hh → src/libutil/unix/signals-impl.hh
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
#pragma once
///@file
/**
* @file
*
* Implementation of some inline definitions for Unix signals, and also
* some extra Unix-only interfaces.
*
* (The only reason everything about signals isn't Unix-only is some
* no-op definitions are provided on Windows to avoid excess CPP in
* downstream code.)
*/

#include "types.hh"
#include "error.hh"
Expand All @@ -24,22 +33,20 @@ namespace nix {

/* User interruption. */

namespace unix {

extern std::atomic<bool> _isInterrupted;

extern thread_local std::function<bool()> interruptCheck;

void setInterruptThrown();

void _interrupted();

void inline checkInterrupt()
{
if (_isInterrupted || (interruptCheck && interruptCheck()))
_interrupted();
}

MakeError(Interrupted, BaseError);

/**
* Sets the signal mask. Like saveSignalMask() but for a signal set that doesn't
* necessarily match the current thread's mask.
* See saveSignalMask() to set the saved mask to the current mask.
*/
void setChildSignalMask(sigset_t *sigs);

/**
* Start a thread that handles various signals. Also block those signals
Expand All @@ -63,26 +70,26 @@ void saveSignalMask();
*/
void restoreSignals();

/**
* Sets the signal mask. Like saveSignalMask() but for a signal set that doesn't
* necessarily match the current thread's mask.
* See saveSignalMask() to set the saved mask to the current mask.
*/
void setChildSignalMask(sigset_t *sigs);
void triggerInterrupt();

struct InterruptCallback
}

static inline void setInterrupted(bool isInterrupted)
{
virtual ~InterruptCallback() { };
};
unix::_isInterrupted = isInterrupted;
}

/**
* Register a function that gets called on SIGINT (in a non-signal
* context).
*/
std::unique_ptr<InterruptCallback> createInterruptCallback(
std::function<void()> callback);
static inline bool getInterrupted()
{
return unix::_isInterrupted;
}

void triggerInterrupt();
void inline checkInterrupt()
{
using namespace unix;
if (_isInterrupted || (interruptCheck && interruptCheck()))
_interrupted();
}

/**
* A RAII class that causes the current thread to receive SIGUSR1 when
Expand Down
23 changes: 14 additions & 9 deletions src/libutil/unix/signals.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,22 @@

namespace nix {

std::atomic<bool> _isInterrupted = false;
using namespace unix;

std::atomic<bool> unix::_isInterrupted = false;

namespace unix {
static thread_local bool interruptThrown = false;
thread_local std::function<bool()> interruptCheck;
}

thread_local std::function<bool()> unix::interruptCheck;

void setInterruptThrown()
{
interruptThrown = true;
unix::interruptThrown = true;
}

void _interrupted()
void unix::_interrupted()
{
/* Block user interrupts while an exception is being handled.
Throwing an exception while another exception is being handled
Expand Down Expand Up @@ -65,7 +70,7 @@ static void signalHandlerThread(sigset_t set)
}
}

void triggerInterrupt()
void unix::triggerInterrupt()
{
_isInterrupted = true;

Expand Down Expand Up @@ -96,7 +101,7 @@ void triggerInterrupt()
static sigset_t savedSignalMask;
static bool savedSignalMaskIsSet = false;

void setChildSignalMask(sigset_t * sigs)
void unix::setChildSignalMask(sigset_t * sigs)
{
assert(sigs); // C style function, but think of sigs as a reference

Expand All @@ -115,14 +120,14 @@ void setChildSignalMask(sigset_t * sigs)
savedSignalMaskIsSet = true;
}

void saveSignalMask() {
void unix::saveSignalMask() {
if (sigprocmask(SIG_BLOCK, nullptr, &savedSignalMask))
throw SysError("querying signal mask");

savedSignalMaskIsSet = true;
}

void startSignalHandlerThread()
void unix::startSignalHandlerThread()
{
updateWindowSize();

Expand All @@ -141,7 +146,7 @@ void startSignalHandlerThread()
std::thread(signalHandlerThread, set).detach();
}

void restoreSignals()
void unix::restoreSignals()
{
// If startSignalHandlerThread wasn't called, that means we're not running
// in a proper libmain process, but a process that presumably manages its
Expand Down

0 comments on commit 1a1448b

Please sign in to comment.