Commit 08a8b17c authored by Marcin Siodelski's avatar Marcin Siodelski

[#796,!504] Avoid memory allocation in signal handler.

parent f858d9d0
// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2014-2019 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -8,6 +8,7 @@
#include <util/signal_set.h>
#include <array>
#include <cerrno>
#include <list>
......@@ -16,6 +17,12 @@ using namespace isc::util;
namespace {
/// @brief Fixed size storage for the codes of received signals.
///
/// It is initialized with all zeros, which means that no signals
/// have been received.
std::array<int, 1024> received_signals;
/// @brief Returns a pointer to the global set of registered signals.
///
/// Multiple instances of @c SignalSet may use this pointer to access
......@@ -72,8 +79,23 @@ void internalHandler(int sig) {
}
}
// First occurrence, so save it.
states->push_back(sig);
// We haven't yet received this signal so we have to record its
// reception. We're using the fixed size array to temporarily
// store the received signal codes. We can't directly insert
// the signal to the 'states' list because this would cause a
// new allocation (malloc) which is not async-signal-safe.
for (int i = 0; i < received_signals.size(); ++i) {
// We have already recorded this signal.
if (received_signals[i] == sig) {
return;
} else if (received_signals[i] == 0) {
// We reached the end of the useful data in the storage.
// Put the signal code into the storage.
received_signals[i] = sig;
break;
}
}
}
/// @brief Optional handler to execute at the time of signal receipt
......@@ -131,21 +153,24 @@ SignalSet::invokeOnReceiptHandler(int sig) {
return (signal_processed);
}
SignalSet::SignalSet(const int sig0) {
SignalSet::SignalSet(const int sig0)
: blocked_(false) {
// Copy static pointers to ensure they don't lose scope before we do.
registered_signals_ = getRegisteredSignals();
signal_states_ = getSignalStates();
add(sig0);
}
SignalSet::SignalSet(const int sig0, const int sig1) {
SignalSet::SignalSet(const int sig0, const int sig1) :
blocked_(false) {
registered_signals_ = getRegisteredSignals();
signal_states_ = getSignalStates();
add(sig0);
add(sig1);
}
SignalSet::SignalSet(const int sig0, const int sig1, const int sig2) {
SignalSet::SignalSet(const int sig0, const int sig1, const int sig2) :
blocked_(false) {
registered_signals_ = getRegisteredSignals();
signal_states_ = getSignalStates();
add(sig0);
......@@ -180,8 +205,9 @@ SignalSet::add(const int sig) {
}
void
SignalSet::block() const {
SignalSet::block() {
maskSignals(SIG_BLOCK);
blocked_ = true;
}
void
......@@ -198,14 +224,49 @@ SignalSet::clear() {
}
int
SignalSet::getNext() const {
SignalSet::getNext() {
// Since we're operating on the global values that the signal
// handler has access to, we have to temporarily block the
// signals to not interfere with unsafe operations performed
// by this method. Remember if the signals were blocked or
// not to be able to determine whether they should be unblocked
// when the function returns.
bool was_blocked = blocked_;
if (!was_blocked) {
block();
}
// We may have some signals received. We need to iterate over those
// and move them to the signal states structure.
for (int i = 0; i < received_signals.size(); ++i) {
// If we hit the end of useful data, break.
if (received_signals[i] == 0) {
break;
}
// Check if such signal has been already recorded.
if (std::find(signal_states_->begin(), signal_states_->end(), received_signals[i])
== signal_states_->end()) {
signal_states_->push_back(received_signals[i]);
}
// Set the current element to 0 to indicate the end of
// the useful data and process the next element.
received_signals[i] = 0;
}
int sig = -1;
for (std::list<int>::iterator it = signal_states_->begin();
it != signal_states_->end(); ++it) {
if (local_signals_.find(*it) != local_signals_.end()) {
return (*it);
sig = *it;
break;
}
}
return (-1);
// If we blocked signals here locally, we have to unblock them.
if (!was_blocked) {
unblock();
}
return (sig);
}
void
......@@ -215,6 +276,18 @@ SignalSet::erase(const int sig) {
<< " from a signal set: signal is not owned by the"
" signal set");
}
// Since we're operating on the global values that the signal
// handler has access to, we have to temporarily block the
// signals to not interfere with unsafe operations performed
// by this method. Remember if the signals were blocked or
// not to be able to determine whether they should be unblocked
// when the function returns.
bool was_blocked = blocked_;
if (!was_blocked) {
block();
}
// Remove globally registered signal.
registered_signals_->erase(sig);
// Remove unhandled signals from the queue.
......@@ -227,6 +300,18 @@ SignalSet::erase(const int sig) {
// Remove locally registered signal.
local_signals_.erase(sig);
// If the signal is no longer supported by this signal set,
// we have to unblock it, because other signal set instance
// may be using it. We won't process it anyway after it is
// removed from this set.
unblock(sig);
// Unblock all remaining signals if we have blocked them in
// this function.
if (!was_blocked) {
unblock();
}
}
void
......@@ -257,12 +342,16 @@ SignalSet::insert(const int sig) {
}
void
SignalSet::maskSignals(const int mask) const {
SignalSet::maskSignals(const int mask, const int sig) const {
sigset_t new_set;
sigemptyset(&new_set);
for (std::set<int>::const_iterator it = registered_signals_->begin();
it != registered_signals_->end(); ++it) {
sigaddset(&new_set, *it);
if (sig < 0) {
for (std::set<int>::const_iterator it = registered_signals_->begin();
it != registered_signals_->end(); ++it) {
sigaddset(&new_set, *it);
}
} else {
sigaddset(&new_set, sig);
}
pthread_sigmask(mask, &new_set, 0);
}
......@@ -298,8 +387,16 @@ SignalSet::remove(const int sig) {
}
void
SignalSet::unblock() const {
maskSignals(SIG_UNBLOCK);
SignalSet::unblock(int sig) {
// There are two cases supported by this function. One is to
// unblock some specific signal. Another one is to unblock all
// signals supported by this function. The maskSignals function
// will deal wih this internally but we marked signals as unblocked
// only if we unblocked all of them (sig is negative).
maskSignals(SIG_UNBLOCK, sig);
if (sig < 0) {
blocked_ = false;
}
}
......
// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2014-2019 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -136,7 +136,7 @@ public:
///
/// @return A code of the next received signal or -1 if there are no
/// more signals received.
int getNext() const;
int getNext();
/// @brief Calls a handler for the next received signal.
///
......@@ -191,7 +191,7 @@ private:
///
/// This function blocks the signals in a set to prevent race condition
/// between the signal handler and the new signal coming in.
void block() const;
void block();
/// @brief Removes the signal from the set.
///
......@@ -215,7 +215,9 @@ private:
/// to apply the SIG_BLOCK and SIG_UNBLOCK mask to signals.
///
/// @param mask A mask to be applied to all signals.
void maskSignals(const int mask) const;
/// @param sig Optional signal to be masked. If this value is negative all
/// signals are masked.
void maskSignals(const int mask, const int sig = -1) const;
/// @brief Pops a next signal number from the static collection of signals.
///
......@@ -227,7 +229,13 @@ private:
/// @brief Unblocks signals in the set.
///
/// This function unblocks the signals in a set.
void unblock() const;
///
/// @param sig Optional signal to be unblocked. If this is negative, all
/// registered signals are unblocked.
void unblock(int sig = -1);
/// @brief Indicates if receiving signals is blocked.
bool blocked_;
/// @brief Stores the set of signals registered in this signal set.
std::set<int> local_signals_;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment