Skip to content

Commit

Permalink
Reland "Delete unused code to handle posix signals in PhysicalSocketS…
Browse files Browse the repository at this point in the history 
…erver"

This is a reland of d2490ae
Earlier link errors were likely a single trybot with corrupted dep files.

Original change's description:
> Delete unused code to handle posix signals in PhysicalSocketServer
>
> Bug: None
> Change-Id: I3abddef4f1af5499f39a8d3f643c779effe9e01d
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175006
> Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
> Commit-Queue: Niels Moller <nisse@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31237}

Bug: webrtc:11571
Change-Id: I7ea14f26a2186a9d51a75493b7280fc0ad6b8c77
Tbr: kwiberg@webrtc.org
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175042
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31251}
  • Loading branch information
Niels Möller authored and Commit Bot committed May 14, 2020
1 parent 2e2f674 commit 6ee6793
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 391 deletions.
235 changes: 0 additions & 235 deletions rtc_base/physical_socket_server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
// "poll" will be used to wait for the signal dispatcher.
#include <poll.h>
#endif
#include <signal.h>
#include <sys/ioctl.h>
#include <sys/select.h>
#include <sys/time.h>
Expand Down Expand Up @@ -959,181 +958,6 @@ class EventDispatcher : public Dispatcher {
CriticalSection crit_;
};

// These two classes use the self-pipe trick to deliver POSIX signals to our
// select loop. This is the only safe, reliable, cross-platform way to do
// non-trivial things with a POSIX signal in an event-driven program (until
// proper pselect() implementations become ubiquitous).

class PosixSignalHandler {
public:
// POSIX only specifies 32 signals, but in principle the system might have
// more and the programmer might choose to use them, so we size our array
// for 128.
static constexpr int kNumPosixSignals = 128;

// There is just a single global instance. (Signal handlers do not get any
// sort of user-defined void * parameter, so they can't access anything that
// isn't global.)
static PosixSignalHandler* Instance() {
static PosixSignalHandler* const instance = new PosixSignalHandler();
return instance;
}

// Returns true if the given signal number is set.
bool IsSignalSet(int signum) const {
RTC_DCHECK(signum < static_cast<int>(arraysize(received_signal_)));
if (signum < static_cast<int>(arraysize(received_signal_))) {
return received_signal_[signum];
} else {
return false;
}
}

// Clears the given signal number.
void ClearSignal(int signum) {
RTC_DCHECK(signum < static_cast<int>(arraysize(received_signal_)));
if (signum < static_cast<int>(arraysize(received_signal_))) {
received_signal_[signum] = false;
}
}

// Returns the file descriptor to monitor for signal events.
int GetDescriptor() const { return afd_[0]; }

// This is called directly from our real signal handler, so it must be
// signal-handler-safe. That means it cannot assume anything about the
// user-level state of the process, since the handler could be executed at any
// time on any thread.
void OnPosixSignalReceived(int signum) {
if (signum >= static_cast<int>(arraysize(received_signal_))) {
// We don't have space in our array for this.
return;
}
// Set a flag saying we've seen this signal.
received_signal_[signum] = true;
// Notify application code that we got a signal.
const uint8_t b[1] = {0};
if (-1 == write(afd_[1], b, sizeof(b))) {
// Nothing we can do here. If there's an error somehow then there's
// nothing we can safely do from a signal handler.
// No, we can't even safely log it.
// But, we still have to check the return value here. Otherwise,
// GCC 4.4.1 complains ignoring return value. Even (void) doesn't help.
return;
}
}

private:
PosixSignalHandler() {
if (pipe(afd_) < 0) {
RTC_LOG_ERR(LS_ERROR) << "pipe failed";
return;
}
if (fcntl(afd_[0], F_SETFL, O_NONBLOCK) < 0) {
RTC_LOG_ERR(LS_WARNING) << "fcntl #1 failed";
}
if (fcntl(afd_[1], F_SETFL, O_NONBLOCK) < 0) {
RTC_LOG_ERR(LS_WARNING) << "fcntl #2 failed";
}
memset(const_cast<void*>(static_cast<volatile void*>(received_signal_)), 0,
sizeof(received_signal_));
}

~PosixSignalHandler() {
int fd1 = afd_[0];
int fd2 = afd_[1];
// We clobber the stored file descriptor numbers here or else in principle
// a signal that happens to be delivered during application termination
// could erroneously write a zero byte to an unrelated file handle in
// OnPosixSignalReceived() if some other file happens to be opened later
// during shutdown and happens to be given the same file descriptor number
// as our pipe had. Unfortunately even with this precaution there is still a
// race where that could occur if said signal happens to be handled
// concurrently with this code and happens to have already read the value of
// afd_[1] from memory before we clobber it, but that's unlikely.
afd_[0] = -1;
afd_[1] = -1;
close(fd1);
close(fd2);
}

int afd_[2];
// These are boolean flags that will be set in our signal handler and read
// and cleared from Wait(). There is a race involved in this, but it is
// benign. The signal handler sets the flag before signaling the pipe, so
// we'll never end up blocking in select() while a flag is still true.
// However, if two of the same signal arrive close to each other then it's
// possible that the second time the handler may set the flag while it's still
// true, meaning that signal will be missed. But the first occurrence of it
// will still be handled, so this isn't a problem.
// Volatile is not necessary here for correctness, but this data _is_ volatile
// so I've marked it as such.
volatile uint8_t received_signal_[kNumPosixSignals];
};

class PosixSignalDispatcher : public Dispatcher {
public:
PosixSignalDispatcher(PhysicalSocketServer* owner) : owner_(owner) {
owner_->Add(this);
}

~PosixSignalDispatcher() override { owner_->Remove(this); }

uint32_t GetRequestedEvents() override { return DE_READ; }

void OnPreEvent(uint32_t ff) override {
// Events might get grouped if signals come very fast, so we read out up to
// 16 bytes to make sure we keep the pipe empty.
uint8_t b[16];
ssize_t ret = read(GetDescriptor(), b, sizeof(b));
if (ret < 0) {
RTC_LOG_ERR(LS_WARNING) << "Error in read()";
} else if (ret == 0) {
RTC_LOG(LS_WARNING) << "Should have read at least one byte";
}
}

void OnEvent(uint32_t ff, int err) override {
for (int signum = 0; signum < PosixSignalHandler::kNumPosixSignals;
++signum) {
if (PosixSignalHandler::Instance()->IsSignalSet(signum)) {
PosixSignalHandler::Instance()->ClearSignal(signum);
HandlerMap::iterator i = handlers_.find(signum);
if (i == handlers_.end()) {
// This can happen if a signal is delivered to our process at around
// the same time as we unset our handler for it. It is not an error
// condition, but it's unusual enough to be worth logging.
RTC_LOG(LS_INFO) << "Received signal with no handler: " << signum;
} else {
// Otherwise, execute our handler.
(*i->second)(signum);
}
}
}
}

int GetDescriptor() override {
return PosixSignalHandler::Instance()->GetDescriptor();
}

bool IsDescriptorClosed() override { return false; }

void SetHandler(int signum, void (*handler)(int)) {
handlers_[signum] = handler;
}

void ClearHandler(int signum) { handlers_.erase(signum); }

bool HasHandlers() { return !handlers_.empty(); }

private:
typedef std::map<int, void (*)(int)> HandlerMap;

HandlerMap handlers_;
// Our owner.
PhysicalSocketServer* owner_;
};

#endif // WEBRTC_POSIX

#if defined(WEBRTC_WIN)
Expand Down Expand Up @@ -1230,9 +1054,6 @@ PhysicalSocketServer::PhysicalSocketServer()
PhysicalSocketServer::~PhysicalSocketServer() {
#if defined(WEBRTC_WIN)
WSACloseEvent(socket_ev_);
#endif
#if defined(WEBRTC_POSIX)
signal_dispatcher_.reset();
#endif
delete signal_wakeup_;
#if defined(WEBRTC_USE_EPOLL)
Expand Down Expand Up @@ -1750,62 +1571,6 @@ bool PhysicalSocketServer::WaitPoll(int cmsWait, Dispatcher* dispatcher) {

#endif // WEBRTC_USE_EPOLL

static void GlobalSignalHandler(int signum) {
PosixSignalHandler::Instance()->OnPosixSignalReceived(signum);
}

bool PhysicalSocketServer::SetPosixSignalHandler(int signum,
void (*handler)(int)) {
// If handler is SIG_IGN or SIG_DFL then clear our user-level handler,
// otherwise set one.
if (handler == SIG_IGN || handler == SIG_DFL) {
if (!InstallSignal(signum, handler)) {
return false;
}
if (signal_dispatcher_) {
signal_dispatcher_->ClearHandler(signum);
if (!signal_dispatcher_->HasHandlers()) {
signal_dispatcher_.reset();
}
}
} else {
if (!signal_dispatcher_) {
signal_dispatcher_.reset(new PosixSignalDispatcher(this));
}
signal_dispatcher_->SetHandler(signum, handler);
if (!InstallSignal(signum, &GlobalSignalHandler)) {
return false;
}
}
return true;
}

Dispatcher* PhysicalSocketServer::signal_dispatcher() {
return signal_dispatcher_.get();
}

bool PhysicalSocketServer::InstallSignal(int signum, void (*handler)(int)) {
struct sigaction act;
// It doesn't really matter what we set this mask to.
if (sigemptyset(&act.sa_mask) != 0) {
RTC_LOG_ERR(LS_ERROR) << "Couldn't set mask";
return false;
}
act.sa_handler = handler;
#if !defined(__native_client__)
// Use SA_RESTART so that our syscalls don't get EINTR, since we don't need it
// and it's a nuisance. Though some syscalls still return EINTR and there's no
// real standard for which ones. :(
act.sa_flags = SA_RESTART;
#else
act.sa_flags = 0;
#endif
if (sigaction(signum, &act, nullptr) != 0) {
RTC_LOG_ERR(LS_ERROR) << "Couldn't set sigaction";
return false;
}
return true;
}
#endif // WEBRTC_POSIX

#if defined(WEBRTC_WIN)
Expand Down
23 changes: 0 additions & 23 deletions rtc_base/physical_socket_server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ enum DispatcherEvent {
};

class Signaler;
#if defined(WEBRTC_POSIX)
class PosixSignalDispatcher;
#endif

class Dispatcher {
public:
Expand Down Expand Up @@ -82,33 +79,13 @@ class RTC_EXPORT PhysicalSocketServer : public SocketServer {
void Remove(Dispatcher* dispatcher);
void Update(Dispatcher* dispatcher);

#if defined(WEBRTC_POSIX)
// Sets the function to be executed in response to the specified POSIX signal.
// The function is executed from inside Wait() using the "self-pipe trick"--
// regardless of which thread receives the signal--and hence can safely
// manipulate user-level data structures.
// "handler" may be SIG_IGN, SIG_DFL, or a user-specified function, just like
// with signal(2).
// Only one PhysicalSocketServer should have user-level signal handlers.
// Dispatching signals on multiple PhysicalSocketServers is not reliable.
// The signal mask is not modified. It is the caller's responsibily to
// maintain it as desired.
virtual bool SetPosixSignalHandler(int signum, void (*handler)(int));

protected:
Dispatcher* signal_dispatcher();
#endif

private:
typedef std::set<Dispatcher*> DispatcherSet;

void AddRemovePendingDispatchers();

#if defined(WEBRTC_POSIX)
bool WaitSelect(int cms, bool process_io);
static bool InstallSignal(int signum, void (*handler)(int));

std::unique_ptr<PosixSignalDispatcher> signal_dispatcher_;
#endif // WEBRTC_POSIX
#if defined(WEBRTC_USE_EPOLL)
void AddEpoll(Dispatcher* dispatcher);
Expand Down
Loading

0 comments on commit 6ee6793

Please sign in to comment.