From 21347458adcb681db99c0eb613ce44d07ea1f0af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Fri, 12 Feb 2021 11:19:52 +0100 Subject: [PATCH] Simplify PhysicalSocketServer Merge OnPreEvent and OnEvent methods. Merge EventDispatcher class into Signaler class. Bug: None Change-Id: I3c07613c76a32a628926569aab0e1076e48a0a79 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206983 Reviewed-by: Harald Alvestrand Reviewed-by: Taylor Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#33262} --- rtc_base/physical_socket_server.cc | 88 ++++++++++++++---------------- rtc_base/physical_socket_server.h | 2 - 2 files changed, 42 insertions(+), 48 deletions(-) diff --git a/rtc_base/physical_socket_server.cc b/rtc_base/physical_socket_server.cc index ee611a1cf7..7904548041 100644 --- a/rtc_base/physical_socket_server.cc +++ b/rtc_base/physical_socket_server.cc @@ -760,21 +760,14 @@ uint32_t SocketDispatcher::GetRequestedEvents() { return enabled_events(); } -void SocketDispatcher::OnPreEvent(uint32_t ff) { - if ((ff & DE_CONNECT) != 0) - state_ = CS_CONNECTED; - -#if defined(WEBRTC_WIN) -// We set CS_CLOSED from CheckSignalClose. -#elif defined(WEBRTC_POSIX) - if ((ff & DE_CLOSE) != 0) - state_ = CS_CLOSED; -#endif -} - #if defined(WEBRTC_WIN) void SocketDispatcher::OnEvent(uint32_t ff, int err) { + if ((ff & DE_CONNECT) != 0) + state_ = CS_CONNECTED; + + // We set CS_CLOSED from CheckSignalClose. + int cache_id = id_; // Make sure we deliver connect/accept first. Otherwise, consumers may see // something like a READ followed by a CONNECT, which would be odd. @@ -809,6 +802,12 @@ void SocketDispatcher::OnEvent(uint32_t ff, int err) { #elif defined(WEBRTC_POSIX) void SocketDispatcher::OnEvent(uint32_t ff, int err) { + if ((ff & DE_CONNECT) != 0) + state_ = CS_CONNECTED; + + if ((ff & DE_CLOSE) != 0) + state_ = CS_CLOSED; + #if defined(WEBRTC_USE_EPOLL) // Remember currently enabled events so we can combine multiple changes // into one update call later. @@ -920,15 +919,25 @@ int SocketDispatcher::Close() { } #if defined(WEBRTC_POSIX) -class EventDispatcher : public Dispatcher { +// Sets the value of a boolean value to false when signaled. +class Signaler : public Dispatcher { public: - EventDispatcher(PhysicalSocketServer* ss) : ss_(ss), fSignaled_(false) { - if (pipe(afd_) < 0) - RTC_LOG(LERROR) << "pipe failed"; + Signaler(PhysicalSocketServer* ss, bool& flag_to_clear) + : ss_(ss), + afd_([] { + std::array afd = {-1, -1}; + + if (pipe(afd.data()) < 0) { + RTC_LOG(LERROR) << "pipe failed"; + } + return afd; + }()), + fSignaled_(false), + flag_to_clear_(flag_to_clear) { ss_->Add(this); } - ~EventDispatcher() override { + ~Signaler() override { ss_->Remove(this); close(afd_[0]); close(afd_[1]); @@ -946,7 +955,7 @@ class EventDispatcher : public Dispatcher { uint32_t GetRequestedEvents() override { return DE_READ; } - void OnPreEvent(uint32_t ff) override { + void OnEvent(uint32_t ff, int err) override { // It is not possible to perfectly emulate an auto-resetting event with // pipes. This simulates it by resetting before the event is handled. @@ -957,19 +966,19 @@ class EventDispatcher : public Dispatcher { RTC_DCHECK_EQ(1, res); fSignaled_ = false; } + flag_to_clear_ = false; } - void OnEvent(uint32_t ff, int err) override { RTC_NOTREACHED(); } - int GetDescriptor() override { return afd_[0]; } bool IsDescriptorClosed() override { return false; } private: PhysicalSocketServer* const ss_; - int afd_[2]; // Assigned in constructor only. + const std::array afd_; bool fSignaled_ RTC_GUARDED_BY(mutex_); webrtc::Mutex mutex_; + bool& flag_to_clear_; }; #endif // WEBRTC_POSIX @@ -988,16 +997,18 @@ static uint32_t FlagsToEvents(uint32_t events) { return ffFD; } -class EventDispatcher : public Dispatcher { +// Sets the value of a boolean value to false when signaled. +class Signaler : public Dispatcher { public: - EventDispatcher(PhysicalSocketServer* ss) : ss_(ss) { + Signaler(PhysicalSocketServer* ss, bool& flag_to_clear) + : ss_(ss), flag_to_clear_(flag_to_clear) { hev_ = WSACreateEvent(); if (hev_) { ss_->Add(this); } } - ~EventDispatcher() override { + ~Signaler() override { if (hev_ != nullptr) { ss_->Remove(this); WSACloseEvent(hev_); @@ -1012,9 +1023,10 @@ class EventDispatcher : public Dispatcher { uint32_t GetRequestedEvents() override { return 0; } - void OnPreEvent(uint32_t ff) override { WSAResetEvent(hev_); } - - void OnEvent(uint32_t ff, int err) override {} + void OnEvent(uint32_t ff, int err) override { + WSAResetEvent(hev_); + flag_to_clear_ = false; + } WSAEVENT GetWSAEvent() override { return hev_; } @@ -1025,24 +1037,10 @@ class EventDispatcher : public Dispatcher { private: PhysicalSocketServer* ss_; WSAEVENT hev_; + bool& flag_to_clear_; }; #endif // WEBRTC_WIN -// Sets the value of a boolean value to false when signaled. -class Signaler : public EventDispatcher { - public: - Signaler(PhysicalSocketServer* ss, bool* pf) : EventDispatcher(ss), pf_(pf) {} - ~Signaler() override {} - - void OnEvent(uint32_t ff, int err) override { - if (pf_) - *pf_ = false; - } - - private: - bool* pf_; -}; - PhysicalSocketServer::PhysicalSocketServer() : #if defined(WEBRTC_USE_EPOLL) @@ -1062,7 +1060,8 @@ PhysicalSocketServer::PhysicalSocketServer() // Note that -1 == INVALID_SOCKET, the alias used by later checks. } #endif - signal_wakeup_ = new Signaler(this, &fWait_); + // The `fWait_` flag to be cleared by the Signaler. + signal_wakeup_ = new Signaler(this, fWait_); } PhysicalSocketServer::~PhysicalSocketServer() { @@ -1230,7 +1229,6 @@ static void ProcessEvents(Dispatcher* dispatcher, // Tell the descriptor about the event. if (ff != 0) { - dispatcher->OnPreEvent(ff); dispatcher->OnEvent(ff, errcode); } } @@ -1634,7 +1632,6 @@ bool PhysicalSocketServer::Wait(int cmsWait, bool process_io) { continue; } Dispatcher* disp = dispatcher_by_key_.at(key); - disp->OnPreEvent(0); disp->OnEvent(0, 0); } else if (process_io) { // Iterate only on the dispatchers whose sockets were passed into @@ -1705,7 +1702,6 @@ bool PhysicalSocketServer::Wait(int cmsWait, bool process_io) { errcode = wsaEvents.iErrorCode[FD_CLOSE_BIT]; } if (ff != 0) { - disp->OnPreEvent(ff); disp->OnEvent(ff, errcode); } } diff --git a/rtc_base/physical_socket_server.h b/rtc_base/physical_socket_server.h index f83bf52487..4b7957eb20 100644 --- a/rtc_base/physical_socket_server.h +++ b/rtc_base/physical_socket_server.h @@ -50,7 +50,6 @@ class Dispatcher { public: virtual ~Dispatcher() {} virtual uint32_t GetRequestedEvents() = 0; - virtual void OnPreEvent(uint32_t ff) = 0; virtual void OnEvent(uint32_t ff, int err) = 0; #if defined(WEBRTC_WIN) virtual WSAEVENT GetWSAEvent() = 0; @@ -238,7 +237,6 @@ class SocketDispatcher : public Dispatcher, public PhysicalSocket { #endif uint32_t GetRequestedEvents() override; - void OnPreEvent(uint32_t ff) override; void OnEvent(uint32_t ff, int err) override; int Close() override;