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 <hta@webrtc.org> Reviewed-by: Taylor <deadbeef@webrtc.org> Commit-Queue: Niels Moller <nisse@webrtc.org> Cr-Commit-Position: refs/heads/master@{#33262}
diff --git a/rtc_base/physical_socket_server.cc b/rtc_base/physical_socket_server.cc index ee611a1..7904548 100644 --- a/rtc_base/physical_socket_server.cc +++ b/rtc_base/physical_socket_server.cc
@@ -760,21 +760,14 @@ 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 @@ #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 @@ } #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<int, 2> 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 @@ 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 @@ 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<int, 2> afd_; bool fSignaled_ RTC_GUARDED_BY(mutex_); webrtc::Mutex mutex_; + bool& flag_to_clear_; }; #endif // WEBRTC_POSIX @@ -988,16 +997,18 @@ 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 @@ 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 @@ 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 @@ // 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 @@ // Tell the descriptor about the event. if (ff != 0) { - dispatcher->OnPreEvent(ff); dispatcher->OnEvent(ff, errcode); } } @@ -1634,7 +1632,6 @@ 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 @@ 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 f83bf52..4b7957e 100644 --- a/rtc_base/physical_socket_server.h +++ b/rtc_base/physical_socket_server.h
@@ -50,7 +50,6 @@ 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 @@ #endif uint32_t GetRequestedEvents() override; - void OnPreEvent(uint32_t ff) override; void OnEvent(uint32_t ff, int err) override; int Close() override;