Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core: improve signal handling #539

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

core: improve signal handling #539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c4ac16d
core: move handling of user signals to the main loop via signalfd
PaideiaDilemma Oct 30, 2024
8773ea3
core: lock forceUpdateTimers
PaideiaDilemma Oct 30, 2024
1803834
core: only attemptRestoreOnDeath when locked
PaideiaDilemma Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/core/Auth.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "Auth.hpp"
#include "hyprlock.hpp"
#include "../helpers/Log.hpp"
#include "src/config/ConfigManager.hpp"
#include "../config/ConfigManager.hpp"

#include <filesystem>
#include <unistd.h>
Expand Down
106 changes: 80 additions & 26 deletions src/core/hyprlock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <sys/wait.h>
#include <sys/poll.h>
#include <sys/mman.h>
#include <sys/signalfd.h>
#include <fcntl.h>
#include <signal.h>
#include <unistd.h>
Expand Down Expand Up @@ -336,18 +337,9 @@ static void handleUnlockSignal(int sig) {
}
}

static void forceUpdateTimers() {
for (auto& t : g_pHyprlock->getTimers()) {
if (t->canForceUpdate()) {
t->call(t);
t->cancel();
}
}
}

static void handleForceUpdateSignal(int sig) {
if (sig == SIGUSR2) {
forceUpdateTimers();
g_pHyprlock->forceUpdateTimers();
}
}

Expand All @@ -356,19 +348,71 @@ static void handlePollTerminate(int sig) {
}

static void handleCriticalSignal(int sig) {
g_pHyprlock->attemptRestoreOnDeath();

// remove our handlers
struct sigaction sa;
sa.sa_handler = SIG_IGN;
sigemptyset(&sa.sa_mask);
sa.sa_flags = 0;
sigaction(SIGABRT, &sa, NULL);
sigaction(SIGSEGV, &sa, NULL);
sigaction(SIGABRT, &sa, NULL);
sigaction(SIGFPE, &sa, NULL);
sigaction(SIGILL, &sa, NULL);
sigaction(SIGBUS, &sa, NULL);

g_pHyprlock->attemptRestoreOnDeath();

abort();
}

static void handleTerminationSignal(int sig) {
g_pHyprlock->m_bTerminate = true;
}

static int setupSignalHandling() {
registerSignalAction(SIGRTMIN, handlePollTerminate);
registerSignalAction(SIGSEGV, handleCriticalSignal);
registerSignalAction(SIGABRT, handleCriticalSignal);
registerSignalAction(SIGFPE, handleCriticalSignal);
registerSignalAction(SIGILL, handleCriticalSignal);
registerSignalAction(SIGBUS, handleCriticalSignal);

sigset_t mask;
sigemptyset(&mask);
sigaddset(&mask, SIGUSR1);
sigaddset(&mask, SIGUSR2);

RASSERT(sigprocmask(SIG_SETMASK, &mask, NULL) == 0, "Failed to set signal mask");

sigemptyset(&mask);
sigaddset(&mask, SIGUSR1);
sigaddset(&mask, SIGUSR2);

int fd = signalfd(-1, &mask, SFD_NONBLOCK | SFD_CLOEXEC);
RASSERT(fd != -1, "Failed to create signalfd");
return fd;
}

void handleUserSignals(int signalfd) {
struct signalfd_siginfo sfdi;
ssize_t rs;
while ((rs = read(signalfd, &sfdi, sizeof(struct signalfd_siginfo)), rs > 0)) {
if (rs != sizeof(struct signalfd_siginfo)) {
Debug::log(ERR, "Failed to read signal fd");
return;
}

Debug::log(TRACE, "Got signal {}", sfdi.ssi_signo);

switch (sfdi.ssi_signo) {
case SIGUSR1: handleUnlockSignal(sfdi.ssi_signo); break;
case SIGUSR2: handleForceUpdateSignal(sfdi.ssi_signo); break;
default: {
Debug::log(WARN, "Unhandled signal {}", sfdi.ssi_signo);
} break;
}
}
}

void CHyprlock::run() {
m_sWaylandState.registry = wl_display_get_registry(m_sWaylandState.display);

Expand All @@ -384,6 +428,8 @@ void CHyprlock::run() {
// gather info about monitors
wl_display_roundtrip(m_sWaylandState.display);

const int SIGNALFD = setupSignalHandling();

g_pRenderer = std::make_unique<CRenderer>();

static auto* const PNOFADEOUT = (Hyprlang::INT* const*)g_pConfigManager->getValuePtr("general:no_fade_out");
Expand Down Expand Up @@ -422,21 +468,20 @@ void CHyprlock::run() {
g_pFingerprint = std::make_unique<CFingerprint>();
std::shared_ptr<sdbus::IConnection> conn = g_pFingerprint->start();

registerSignalAction(SIGUSR1, handleUnlockSignal, SA_RESTART);
registerSignalAction(SIGUSR2, handleForceUpdateSignal);
registerSignalAction(SIGRTMIN, handlePollTerminate);
registerSignalAction(SIGSEGV, handleCriticalSignal);
registerSignalAction(SIGABRT, handleCriticalSignal);

createSessionLockSurfaces();

pollfd pollfds[2];
pollfd pollfds[3];

pollfds[0] = {
.fd = wl_display_get_fd(m_sWaylandState.display),
.events = POLLIN,
};
pollfds[1] = {
.fd = SIGNALFD,
.events = POLLIN,
};
if (conn) {
pollfds[1] = {
pollfds[2] = {
.fd = conn->getEventLoopPollData().fd,
.events = POLLIN,
};
Expand Down Expand Up @@ -520,6 +565,9 @@ void CHyprlock::run() {

m_sLoopState.event = false;

if (pollfds[2].revents & POLLIN /* signal */)
handleUserSignals(SIGNALFD);

if (pollfds[1].revents & POLLIN /* dbus */) {
while (conn && conn->processPendingEvent()) {
;
Expand Down Expand Up @@ -742,7 +790,7 @@ static void handleKeyboardModifiers(void* data, wl_keyboard* wl_keyboard, uint s

if (group != g_pHyprlock->m_uiActiveLayout) {
g_pHyprlock->m_uiActiveLayout = group;
forceUpdateTimers();
g_pHyprlock->forceUpdateTimers();
}

xkb_state_update_mask(g_pHyprlock->m_pXKBState, mods_depressed, mods_latched, mods_locked, 0, 0, group);
Expand Down Expand Up @@ -1073,12 +1121,18 @@ std::shared_ptr<CTimer> CHyprlock::addTimer(const std::chrono::system_clock::dur
return T;
}

std::vector<std::shared_ptr<CTimer>> CHyprlock::getTimers() {
return m_vTimers;
void CHyprlock::forceUpdateTimers() {
std::lock_guard<std::mutex> lg(m_sLoopState.timersMutex);
for (auto& t : m_vTimers) {
if (t->canForceUpdate()) {
t->call(t);
t->cancel();
}
}
}

void CHyprlock::enqueueForceUpdateTimers() {
addTimer(std::chrono::milliseconds(1), [](std::shared_ptr<CTimer> self, void* data) { forceUpdateTimers(); }, nullptr, false);
addTimer(std::chrono::milliseconds(1), [this](std::shared_ptr<CTimer> self, void* data) { forceUpdateTimers(); }, nullptr, false);
}

void CHyprlock::spawnAsync(const std::string& args) {
Expand Down Expand Up @@ -1156,7 +1210,7 @@ zwlr_screencopy_manager_v1* CHyprlock::getScreencopy() {
}

void CHyprlock::attemptRestoreOnDeath() {
if (m_bTerminate || m_sCurrentDesktop != "Hyprland")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any Idea why we didn't try to restore when m_bTerminate is true before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we want to check if we are actually locked instead?
Cause if we aren't there is no point in trying to restore.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhhh cuz if it crashes after nlock

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah but terminate does not mean we already unlocked

Copy link
Member

@vaxerski vaxerski Nov 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh then ok idk

if (!m_bLocked || m_sCurrentDesktop != "Hyprland")
return;

const auto XDG_RUNTIME_DIR = getenv("XDG_RUNTIME_DIR");
Expand Down
2 changes: 1 addition & 1 deletion src/core/hyprlock.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class CHyprlock {
bool force = false);

void enqueueForceUpdateTimers();
void forceUpdateTimers();

void onLockLocked();
void onLockFinished();
Expand Down Expand Up @@ -115,7 +116,6 @@ class CHyprlock {
std::shared_ptr<CTimer> m_pKeyRepeatTimer = nullptr;

std::vector<std::unique_ptr<COutput>> m_vOutputs;
std::vector<std::shared_ptr<CTimer>> getTimers();

struct {
void* linuxDmabuf = nullptr;
Expand Down
4 changes: 2 additions & 2 deletions src/renderer/AsyncResourceGatherer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
#include "../helpers/MiscFunctions.hpp"
#include "../helpers/Jpeg.hpp"
#include "../helpers/Webp.hpp"
#include "src/helpers/Color.hpp"
#include "src/helpers/Log.hpp"
#include "../helpers/Color.hpp"
#include "../helpers/Log.hpp"

CAsyncResourceGatherer::CAsyncResourceGatherer() {
if (g_pHyprlock->getScreencopy())
Expand Down
Loading