Skip to content

Commit

Permalink
Merge pull request #10401 from nix-windows/better-signals-interface
Browse files Browse the repository at this point in the history
Better signals interface
  • Loading branch information
edolstra authored Apr 5, 2024
2 parents 3d0d908 + 50f621b commit 75fd09b
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 75fd09b

Please sign in to comment.