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

Side by Side Diff: base/waitable_event_watcher_posix.cc

Issue 53026: POSIX: allow WaitableEvents to be deleted while watching them. (Closed)
Patch Set: Addressing Jeremy's comments Created 11 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/waitable_event_watcher.h ('k') | base/waitable_event_watcher_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) 2006-2008 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2006-2008 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 "base/waitable_event_watcher.h" 5 #include "base/waitable_event_watcher.h"
6 6
7 #include "base/condition_variable.h" 7 #include "base/condition_variable.h"
8 #include "base/lock.h" 8 #include "base/lock.h"
9 #include "base/message_loop.h" 9 #include "base/message_loop.h"
10 #include "base/waitable_event.h" 10 #include "base/waitable_event.h"
(...skipping 137 matching lines...) Expand 10 before | Expand all | Expand 10 after
148 message_loop_ = NULL; 148 message_loop_ = NULL;
149 } 149 }
150 150
151 cancel_flag_ = NULL; 151 cancel_flag_ = NULL;
152 } 152 }
153 153
154 DCHECK(!cancel_flag_.get()) << "StartWatching called while still watching"; 154 DCHECK(!cancel_flag_.get()) << "StartWatching called while still watching";
155 155
156 cancel_flag_ = new Flag; 156 cancel_flag_ = new Flag;
157 callback_task_ = new AsyncCallbackTask(cancel_flag_, delegate, event); 157 callback_task_ = new AsyncCallbackTask(cancel_flag_, delegate, event);
158 WaitableEvent::WaitableEventKernel* kernel = event->kernel_.get();
158 159
159 AutoLock locked(event->lock_); 160 AutoLock locked(kernel->lock_);
160 161
161 if (event->signaled_) { 162 if (kernel->signaled_) {
162 if (!event->manual_reset_) 163 if (!kernel->manual_reset_)
163 event->signaled_ = false; 164 kernel->signaled_ = false;
164 165
165 // No hairpinning - we can't call the delegate directly here. We have to 166 // No hairpinning - we can't call the delegate directly here. We have to
166 // enqueue a task on the MessageLoop as normal. 167 // enqueue a task on the MessageLoop as normal.
167 current_ml->PostTask(FROM_HERE, callback_task_); 168 current_ml->PostTask(FROM_HERE, callback_task_);
168 return true; 169 return true;
169 } 170 }
170 171
171 message_loop_ = current_ml; 172 message_loop_ = current_ml;
172 current_ml->AddDestructionObserver(this); 173 current_ml->AddDestructionObserver(this);
173 174
174 event_ = event; 175 event_ = event;
176 kernel_ = kernel;
175 waiter_ = new AsyncWaiter(current_ml, callback_task_, cancel_flag_); 177 waiter_ = new AsyncWaiter(current_ml, callback_task_, cancel_flag_);
176 event->Enqueue(waiter_); 178 event->Enqueue(waiter_);
177 179
178 return true; 180 return true;
179 } 181 }
180 182
181 void WaitableEventWatcher::StopWatching() { 183 void WaitableEventWatcher::StopWatching() {
182 if (message_loop_) { 184 if (message_loop_) {
183 message_loop_->RemoveDestructionObserver(this); 185 message_loop_->RemoveDestructionObserver(this);
184 message_loop_ = NULL; 186 message_loop_ = NULL;
185 } 187 }
186 188
187 if (!cancel_flag_.get()) // if not currently watching... 189 if (!cancel_flag_.get()) // if not currently watching...
188 return; 190 return;
189 191
190 if (cancel_flag_->value()) { 192 if (cancel_flag_->value()) {
191 // In this case, the event has fired, but we haven't figured that out yet. 193 // In this case, the event has fired, but we haven't figured that out yet.
192 // The WaitableEvent may have been deleted too. 194 // The WaitableEvent may have been deleted too.
193 cancel_flag_ = NULL; 195 cancel_flag_ = NULL;
194 return; 196 return;
195 } 197 }
196 198
197 if (!event_) { 199 if (!kernel_.get()) {
198 // We have no WaitableEvent. This means that we never enqueued a Waiter on 200 // We have no kernel. This means that we never enqueued a Waiter on an
199 // an event because the event was already signaled when StartWatching was 201 // event because the event was already signaled when StartWatching was
200 // called. 202 // called.
201 // 203 //
202 // In this case, a task was enqueued on the MessageLoop and will run. 204 // In this case, a task was enqueued on the MessageLoop and will run.
203 // We set the flag in case the task hasn't yet run. The flag will stop the 205 // We set the flag in case the task hasn't yet run. The flag will stop the
204 // delegate getting called. If the task has run then we have the last 206 // delegate getting called. If the task has run then we have the last
205 // reference to the flag and it will be deleted immedately after. 207 // reference to the flag and it will be deleted immedately after.
206 cancel_flag_->Set(); 208 cancel_flag_->Set();
207 cancel_flag_ = NULL; 209 cancel_flag_ = NULL;
208 return; 210 return;
209 } 211 }
210 212
211 AutoLock locked(event_->lock_); 213 AutoLock locked(kernel_->lock_);
212 // We have a lock on the WaitableEvent. No one else can signal the event while 214 // We have a lock on the kernel. No one else can signal the event while we
213 // we have it. 215 // have it.
214 216
215 // We have a possible ABA issue here. If Dequeue was to compare only the 217 // We have a possible ABA issue here. If Dequeue was to compare only the
216 // pointer values then it's possible that the AsyncWaiter could have been 218 // pointer values then it's possible that the AsyncWaiter could have been
217 // fired, freed and the memory reused for a different Waiter which was 219 // fired, freed and the memory reused for a different Waiter which was
218 // enqueued in the same wait-list. We would think that that waiter was our 220 // enqueued in the same wait-list. We would think that that waiter was our
219 // AsyncWaiter and remove it. 221 // AsyncWaiter and remove it.
220 // 222 //
221 // To stop this, Dequeue also takes a tag argument which is passed to the 223 // To stop this, Dequeue also takes a tag argument which is passed to the
222 // virtual Compare function before the two are considered a match. So we need 224 // virtual Compare function before the two are considered a match. So we need
223 // a tag which is good for the lifetime of this handle: the Flag. Since we 225 // a tag which is good for the lifetime of this handle: the Flag. Since we
224 // have a reference to the Flag, its memory cannot be reused while this object 226 // have a reference to the Flag, its memory cannot be reused while this object
225 // still exists. So if we find a waiter with the correct pointer value, and 227 // still exists. So if we find a waiter with the correct pointer value, and
226 // which shares a Flag pointer, we have a real match. 228 // which shares a Flag pointer, we have a real match.
227 if (event_->Dequeue(waiter_, cancel_flag_.get())) { 229 if (kernel_->Dequeue(waiter_, cancel_flag_.get())) {
228 // Case 2: the waiter hasn't been signaled yet; it was still on the wait 230 // Case 2: the waiter hasn't been signaled yet; it was still on the wait
229 // list. We've removed it, thus we can delete it and the task (which cannot 231 // list. We've removed it, thus we can delete it and the task (which cannot
230 // have been enqueued with the MessageLoop because the waiter was never 232 // have been enqueued with the MessageLoop because the waiter was never
231 // signaled) 233 // signaled)
232 delete waiter_; 234 delete waiter_;
233 delete callback_task_; 235 delete callback_task_;
234 cancel_flag_ = NULL; 236 cancel_flag_ = NULL;
235 return; 237 return;
236 } 238 }
237 239
(...skipping 26 matching lines...) Expand all
264 // ----------------------------------------------------------------------------- 266 // -----------------------------------------------------------------------------
265 // This is called when the MessageLoop which the callback will be run it is 267 // This is called when the MessageLoop which the callback will be run it is
266 // deleted. We need to cancel the callback as if we had been deleted, but we 268 // deleted. We need to cancel the callback as if we had been deleted, but we
267 // will still be deleted at some point in the future. 269 // will still be deleted at some point in the future.
268 // ----------------------------------------------------------------------------- 270 // -----------------------------------------------------------------------------
269 void WaitableEventWatcher::WillDestroyCurrentMessageLoop() { 271 void WaitableEventWatcher::WillDestroyCurrentMessageLoop() {
270 StopWatching(); 272 StopWatching();
271 } 273 }
272 274
273 } // namespace base 275 } // namespace base
OLDNEW
« no previous file with comments | « base/waitable_event_watcher.h ('k') | base/waitable_event_watcher_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698