1*635a8641SAndroid Build Coastguard WorkerFrom e18f110107399803dd070088c601f9a5540a2a3f Mon Sep 17 00:00:00 2001 2*635a8641SAndroid Build Coastguard WorkerFrom: Hidehiko Abe <[email protected]> 3*635a8641SAndroid Build Coastguard WorkerDate: Thu, 3 Oct 2019 02:04:53 +0900 4*635a8641SAndroid Build Coastguard WorkerSubject: [PATCH] components/timers: fix fd leak in AlarmTimer 5*635a8641SAndroid Build Coastguard Worker 6*635a8641SAndroid Build Coastguard WorkerThis is fixed upstream but will take a while to roll back 7*635a8641SAndroid Build Coastguard Workerinto Chrome OS. 8*635a8641SAndroid Build Coastguard Worker 9*635a8641SAndroid Build Coastguard WorkerBUG=chromium:984593 10*635a8641SAndroid Build Coastguard WorkerTEST=enable/disable wifi repeatedly and see that there is no 11*635a8641SAndroid Build Coastguard Worker growth of open timerfds 12*635a8641SAndroid Build Coastguard Worker 13*635a8641SAndroid Build Coastguard WorkerChange-Id: If2af8f8ddf6b9dc31cda36fe5f5454ca0c5819de 14*635a8641SAndroid Build Coastguard Worker--- 15*635a8641SAndroid Build Coastguard Worker components/timers/alarm_timer_chromeos.cc | 8 ++++---- 16*635a8641SAndroid Build Coastguard Worker components/timers/alarm_timer_chromeos.h | 15 ++++++++------- 17*635a8641SAndroid Build Coastguard Worker 2 files changed, 12 insertions(+), 11 deletions(-) 18*635a8641SAndroid Build Coastguard Worker 19*635a8641SAndroid Build Coastguard Workerdiff --git a/components/timers/alarm_timer_chromeos.cc b/components/timers/alarm_timer_chromeos.cc 20*635a8641SAndroid Build Coastguard Workerindex 0b43134..f14d889 100644 21*635a8641SAndroid Build Coastguard Worker--- a/components/timers/alarm_timer_chromeos.cc 22*635a8641SAndroid Build Coastguard Worker+++ b/components/timers/alarm_timer_chromeos.cc 23*635a8641SAndroid Build Coastguard Worker@@ -80,7 +80,7 @@ void SimpleAlarmTimer::Reset() { 24*635a8641SAndroid Build Coastguard Worker alarm_time.it_value.tv_nsec = 25*635a8641SAndroid Build Coastguard Worker (delay.InMicroseconds() % base::Time::kMicrosecondsPerSecond) * 26*635a8641SAndroid Build Coastguard Worker base::Time::kNanosecondsPerMicrosecond; 27*635a8641SAndroid Build Coastguard Worker- if (timerfd_settime(alarm_fd_, 0, &alarm_time, NULL) < 0) 28*635a8641SAndroid Build Coastguard Worker+ if (timerfd_settime(alarm_fd_.get(), 0, &alarm_time, NULL) < 0) 29*635a8641SAndroid Build Coastguard Worker PLOG(ERROR) << "Error while setting alarm time. Timer will not fire"; 30*635a8641SAndroid Build Coastguard Worker 31*635a8641SAndroid Build Coastguard Worker // The timer is running. 32*635a8641SAndroid Build Coastguard Worker@@ -97,7 +97,7 @@ void SimpleAlarmTimer::Reset() { 33*635a8641SAndroid Build Coastguard Worker base::debug::TaskAnnotator().WillQueueTask("SimpleAlarmTimer::Reset", 34*635a8641SAndroid Build Coastguard Worker pending_task_.get()); 35*635a8641SAndroid Build Coastguard Worker alarm_fd_watcher_ = base::FileDescriptorWatcher::WatchReadable( 36*635a8641SAndroid Build Coastguard Worker- alarm_fd_, 37*635a8641SAndroid Build Coastguard Worker+ alarm_fd_.get(), 38*635a8641SAndroid Build Coastguard Worker base::BindRepeating(&SimpleAlarmTimer::OnAlarmFdReadableWithoutBlocking, 39*635a8641SAndroid Build Coastguard Worker weak_factory_.GetWeakPtr())); 40*635a8641SAndroid Build Coastguard Worker } 41*635a8641SAndroid Build Coastguard Worker@@ -109,7 +109,7 @@ void SimpleAlarmTimer::OnAlarmFdReadableWithoutBlocking() { 42*635a8641SAndroid Build Coastguard Worker 43*635a8641SAndroid Build Coastguard Worker // Read from |alarm_fd_| to ack the event. 44*635a8641SAndroid Build Coastguard Worker char val[sizeof(uint64_t)]; 45*635a8641SAndroid Build Coastguard Worker- if (!base::ReadFromFD(alarm_fd_, val, sizeof(uint64_t))) 46*635a8641SAndroid Build Coastguard Worker+ if (!base::ReadFromFD(alarm_fd_.get(), val, sizeof(uint64_t))) 47*635a8641SAndroid Build Coastguard Worker PLOG(DFATAL) << "Unable to read from timer file descriptor."; 48*635a8641SAndroid Build Coastguard Worker 49*635a8641SAndroid Build Coastguard Worker OnTimerFired(); 50*635a8641SAndroid Build Coastguard Worker@@ -137,7 +137,7 @@ void SimpleAlarmTimer::OnTimerFired() { 51*635a8641SAndroid Build Coastguard Worker } 52*635a8641SAndroid Build Coastguard Worker 53*635a8641SAndroid Build Coastguard Worker bool SimpleAlarmTimer::CanWakeFromSuspend() const { 54*635a8641SAndroid Build Coastguard Worker- return alarm_fd_ != -1; 55*635a8641SAndroid Build Coastguard Worker+ return alarm_fd_.is_valid(); 56*635a8641SAndroid Build Coastguard Worker } 57*635a8641SAndroid Build Coastguard Worker 58*635a8641SAndroid Build Coastguard Worker } // namespace timers 59*635a8641SAndroid Build Coastguard Workerdiff --git a/components/timers/alarm_timer_chromeos.h b/components/timers/alarm_timer_chromeos.h 60*635a8641SAndroid Build Coastguard Workerindex 1ff689e..100ba81 100644 61*635a8641SAndroid Build Coastguard Worker--- a/components/timers/alarm_timer_chromeos.h 62*635a8641SAndroid Build Coastguard Worker+++ b/components/timers/alarm_timer_chromeos.h 63*635a8641SAndroid Build Coastguard Worker@@ -8,6 +8,7 @@ 64*635a8641SAndroid Build Coastguard Worker #include <memory> 65*635a8641SAndroid Build Coastguard Worker 66*635a8641SAndroid Build Coastguard Worker #include "base/files/file_descriptor_watcher_posix.h" 67*635a8641SAndroid Build Coastguard Worker+#include "base/files/scoped_file.h" 68*635a8641SAndroid Build Coastguard Worker #include "base/macros.h" 69*635a8641SAndroid Build Coastguard Worker #include "base/memory/scoped_refptr.h" 70*635a8641SAndroid Build Coastguard Worker #include "base/memory/weak_ptr.h" 71*635a8641SAndroid Build Coastguard Worker@@ -43,6 +44,12 @@ class SimpleAlarmTimer : public base::RetainingOneShotTimer { 72*635a8641SAndroid Build Coastguard Worker void Stop() override; 73*635a8641SAndroid Build Coastguard Worker void Reset() override; 74*635a8641SAndroid Build Coastguard Worker 75*635a8641SAndroid Build Coastguard Worker+ // Tracks whether the timer has the ability to wake the system up from 76*635a8641SAndroid Build Coastguard Worker+ // suspend. This is a runtime check because we won't know if the system 77*635a8641SAndroid Build Coastguard Worker+ // supports being woken up from suspend until the constructor actually tries 78*635a8641SAndroid Build Coastguard Worker+ // to set it up. 79*635a8641SAndroid Build Coastguard Worker+ bool CanWakeFromSuspend() const; 80*635a8641SAndroid Build Coastguard Worker+ 81*635a8641SAndroid Build Coastguard Worker private: 82*635a8641SAndroid Build Coastguard Worker // Called when |alarm_fd_| is readable without blocking. Reads data from 83*635a8641SAndroid Build Coastguard Worker // |alarm_fd_| and calls OnTimerFired(). 84*635a8641SAndroid Build Coastguard Worker@@ -51,14 +58,8 @@ class SimpleAlarmTimer : public base::RetainingOneShotTimer { 85*635a8641SAndroid Build Coastguard Worker // Called when the timer fires. Runs the callback. 86*635a8641SAndroid Build Coastguard Worker void OnTimerFired(); 87*635a8641SAndroid Build Coastguard Worker 88*635a8641SAndroid Build Coastguard Worker- // Tracks whether the timer has the ability to wake the system up from 89*635a8641SAndroid Build Coastguard Worker- // suspend. This is a runtime check because we won't know if the system 90*635a8641SAndroid Build Coastguard Worker- // supports being woken up from suspend until the constructor actually tries 91*635a8641SAndroid Build Coastguard Worker- // to set it up. 92*635a8641SAndroid Build Coastguard Worker- bool CanWakeFromSuspend() const; 93*635a8641SAndroid Build Coastguard Worker- 94*635a8641SAndroid Build Coastguard Worker // Timer file descriptor. 95*635a8641SAndroid Build Coastguard Worker- const int alarm_fd_; 96*635a8641SAndroid Build Coastguard Worker+ base::ScopedFD alarm_fd_; 97*635a8641SAndroid Build Coastguard Worker 98*635a8641SAndroid Build Coastguard Worker // Watches |alarm_fd_|. 99*635a8641SAndroid Build Coastguard Worker std::unique_ptr<base::FileDescriptorWatcher::Controller> alarm_fd_watcher_; 100*635a8641SAndroid Build Coastguard Worker-- 101*635a8641SAndroid Build Coastguard Worker2.23.0.581.g78d2f28ef7-goog 102*635a8641SAndroid Build Coastguard Worker 103