Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(585)

Side by Side Diff: base/synchronization/waitable_event_posix.cc

Issue 999753005: Make sure that WaitableEvent::TimedWait works fine with large timeouts. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Nits Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « base/synchronization/waitable_event.h ('k') | base/synchronization/waitable_event_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include <algorithm> 5 #include <algorithm>
6 #include <vector> 6 #include <vector>
7 7
8 #include "base/logging.h" 8 #include "base/logging.h"
9 #include "base/synchronization/waitable_event.h" 9 #include "base/synchronization/waitable_event.h"
10 #include "base/synchronization/condition_variable.h" 10 #include "base/synchronization/condition_variable.h"
(...skipping 133 matching lines...) Expand 10 before | Expand all | Expand 10 after
144 } 144 }
145 145
146 private: 146 private:
147 bool fired_; 147 bool fired_;
148 WaitableEvent* signaling_event_; // The WaitableEvent which woke us 148 WaitableEvent* signaling_event_; // The WaitableEvent which woke us
149 base::Lock lock_; 149 base::Lock lock_;
150 base::ConditionVariable cv_; 150 base::ConditionVariable cv_;
151 }; 151 };
152 152
153 void WaitableEvent::Wait() { 153 void WaitableEvent::Wait() {
154 bool result = TimedWait(TimeDelta::FromSeconds(-1)); 154 bool result = TimedWait(TimeDelta::Max());
155 DCHECK(result) << "TimedWait() should never fail with infinite timeout"; 155 DCHECK(result) << "TimedWait() should never fail with infinite timeout";
156 } 156 }
157 157
158 bool WaitableEvent::TimedWait(const TimeDelta& max_time) { 158 bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
159 DCHECK_GE(max_time, TimeDelta());
159 base::ThreadRestrictions::AssertWaitAllowed(); 160 base::ThreadRestrictions::AssertWaitAllowed();
160 const TimeTicks end_time(TimeTicks::Now() + max_time); 161 const TimeTicks end_time(TimeTicks::Now() + max_time);
161 const bool finite_time = max_time.ToInternalValue() >= 0;
162 162
163 kernel_->lock_.Acquire(); 163 kernel_->lock_.Acquire();
164 if (kernel_->signaled_) { 164 if (kernel_->signaled_) {
165 if (!kernel_->manual_reset_) { 165 if (!kernel_->manual_reset_) {
166 // In this case we were signaled when we had no waiters. Now that 166 // In this case we were signaled when we had no waiters. Now that
167 // someone has waited upon us, we can automatically reset. 167 // someone has waited upon us, we can automatically reset.
168 kernel_->signaled_ = false; 168 kernel_->signaled_ = false;
169 } 169 }
170 170
171 kernel_->lock_.Release(); 171 kernel_->lock_.Release();
172 return true; 172 return true;
173 } 173 }
174 174
175 SyncWaiter sw; 175 SyncWaiter sw;
176 sw.lock()->Acquire(); 176 sw.lock()->Acquire();
177 177
178 Enqueue(&sw); 178 Enqueue(&sw);
179 kernel_->lock_.Release(); 179 kernel_->lock_.Release();
180 // We are violating locking order here by holding the SyncWaiter lock but not 180 // We are violating locking order here by holding the SyncWaiter lock but not
181 // the WaitableEvent lock. However, this is safe because we don't lock @lock_ 181 // the WaitableEvent lock. However, this is safe because we don't lock @lock_
182 // again before unlocking it. 182 // again before unlocking it.
183 183
184 for (;;) { 184 for (;;) {
185 const TimeTicks current_time(TimeTicks::Now()); 185 const TimeTicks current_time(TimeTicks::Now());
186 186
187 if (sw.fired() || (finite_time && current_time >= end_time)) { 187 if (sw.fired() || current_time >= end_time) {
188 const bool return_value = sw.fired(); 188 const bool return_value = sw.fired();
189 189
190 // We can't acquire @lock_ before releasing the SyncWaiter lock (because 190 // We can't acquire @lock_ before releasing the SyncWaiter lock (because
191 // of locking order), however, in between the two a signal could be fired 191 // of locking order), however, in between the two a signal could be fired
192 // and @sw would accept it, however we will still return false, so the 192 // and @sw would accept it, however we will still return false, so the
193 // signal would be lost on an auto-reset WaitableEvent. Thus we call 193 // signal would be lost on an auto-reset WaitableEvent. Thus we call
194 // Disable which makes sw::Fire return false. 194 // Disable which makes sw::Fire return false.
195 sw.Disable(); 195 sw.Disable();
196 sw.lock()->Release(); 196 sw.lock()->Release();
197 197
198 // This is a bug that has been enshrined in the interface of 198 // This is a bug that has been enshrined in the interface of
199 // WaitableEvent now: |Dequeue| is called even when |sw.fired()| is true, 199 // WaitableEvent now: |Dequeue| is called even when |sw.fired()| is true,
200 // even though it'll always return false in that case. However, taking 200 // even though it'll always return false in that case. However, taking
201 // the lock ensures that |Signal| has completed before we return and 201 // the lock ensures that |Signal| has completed before we return and
202 // means that a WaitableEvent can synchronise its own destruction. 202 // means that a WaitableEvent can synchronise its own destruction.
203 kernel_->lock_.Acquire(); 203 kernel_->lock_.Acquire();
204 kernel_->Dequeue(&sw, &sw); 204 kernel_->Dequeue(&sw, &sw);
205 kernel_->lock_.Release(); 205 kernel_->lock_.Release();
206 206
207 return return_value; 207 return return_value;
208 } 208 }
209 209
210 if (finite_time) { 210 sw.cv()->TimedWait(end_time - current_time);
211 const TimeDelta max_wait(end_time - current_time);
212 sw.cv()->TimedWait(max_wait);
213 } else {
214 sw.cv()->Wait();
215 }
216 } 211 }
217 } 212 }
218 213
219 // ----------------------------------------------------------------------------- 214 // -----------------------------------------------------------------------------
220 // Synchronous waiting on multiple objects. 215 // Synchronous waiting on multiple objects.
221 216
222 static bool // StrictWeakOrdering 217 static bool // StrictWeakOrdering
223 cmp_fst_addr(const std::pair<WaitableEvent*, unsigned> &a, 218 cmp_fst_addr(const std::pair<WaitableEvent*, unsigned> &a,
224 const std::pair<WaitableEvent*, unsigned> &b) { 219 const std::pair<WaitableEvent*, unsigned> &b) {
225 return a.first < b.first; 220 return a.first < b.first;
(...skipping 180 matching lines...) Expand 10 before | Expand all | Expand 10 after
406 return true; 401 return true;
407 } 402 }
408 } 403 }
409 404
410 return false; 405 return false;
411 } 406 }
412 407
413 // ----------------------------------------------------------------------------- 408 // -----------------------------------------------------------------------------
414 409
415 } // namespace base 410 } // namespace base
OLDNEW
« no previous file with comments | « base/synchronization/waitable_event.h ('k') | base/synchronization/waitable_event_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698