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

Side by Side Diff: ipc/ipc_channel_proxy.cc

Issue 194923004: Revert of Eliminate a potential race in IPC::ChannelProxy (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Created 6 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 | « ipc/ipc_channel_proxy.h ('k') | no next file » | 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 "base/bind.h" 5 #include "base/bind.h"
6 #include "base/compiler_specific.h" 6 #include "base/compiler_specific.h"
7 #include "base/debug/trace_event.h" 7 #include "base/debug/trace_event.h"
8 #include "base/location.h" 8 #include "base/location.h"
9 #include "base/memory/ref_counted.h" 9 #include "base/memory/ref_counted.h"
10 #include "base/memory/scoped_ptr.h" 10 #include "base/memory/scoped_ptr.h"
(...skipping 138 matching lines...) Expand 10 before | Expand all | Expand 10 after
149 149
150 ChannelProxy::Context::~Context() { 150 ChannelProxy::Context::~Context() {
151 } 151 }
152 152
153 void ChannelProxy::Context::ClearIPCTaskRunner() { 153 void ChannelProxy::Context::ClearIPCTaskRunner() {
154 ipc_task_runner_ = NULL; 154 ipc_task_runner_ = NULL;
155 } 155 }
156 156
157 void ChannelProxy::Context::CreateChannel(const IPC::ChannelHandle& handle, 157 void ChannelProxy::Context::CreateChannel(const IPC::ChannelHandle& handle,
158 const Channel::Mode& mode) { 158 const Channel::Mode& mode) {
159 DCHECK(!channel_); 159 DCHECK(channel_.get() == NULL);
160 channel_id_ = handle.name; 160 channel_id_ = handle.name;
161 channel_.reset(new Channel(handle, mode, this)); 161 channel_.reset(new Channel(handle, mode, this));
162 } 162 }
163 163
164 bool ChannelProxy::Context::TryFilters(const Message& message) { 164 bool ChannelProxy::Context::TryFilters(const Message& message) {
165 DCHECK(message_filter_router_); 165 DCHECK(message_filter_router_);
166 #ifdef IPC_MESSAGE_LOG_ENABLED 166 #ifdef IPC_MESSAGE_LOG_ENABLED
167 Logging* logger = Logging::GetInstance(); 167 Logging* logger = Logging::GetInstance();
168 if (logger->Enabled()) 168 if (logger->Enabled())
169 logger->OnPreDispatchMessage(message); 169 logger->OnPreDispatchMessage(message);
(...skipping 19 matching lines...) Expand all
189 189
190 // Called on the IPC::Channel thread 190 // Called on the IPC::Channel thread
191 bool ChannelProxy::Context::OnMessageReceivedNoFilter(const Message& message) { 191 bool ChannelProxy::Context::OnMessageReceivedNoFilter(const Message& message) {
192 listener_task_runner_->PostTask( 192 listener_task_runner_->PostTask(
193 FROM_HERE, base::Bind(&Context::OnDispatchMessage, this, message)); 193 FROM_HERE, base::Bind(&Context::OnDispatchMessage, this, message));
194 return true; 194 return true;
195 } 195 }
196 196
197 // Called on the IPC::Channel thread 197 // Called on the IPC::Channel thread
198 void ChannelProxy::Context::OnChannelConnected(int32 peer_pid) { 198 void ChannelProxy::Context::OnChannelConnected(int32 peer_pid) {
199 // We cache off the peer_pid so it can be safely accessed from both threads.
200 peer_pid_ = channel_->peer_pid();
201
202 // Add any pending filters. This avoids a race condition where someone 199 // Add any pending filters. This avoids a race condition where someone
203 // creates a ChannelProxy, calls AddFilter, and then right after starts the 200 // creates a ChannelProxy, calls AddFilter, and then right after starts the
204 // peer process. The IO thread could receive a message before the task to add 201 // peer process. The IO thread could receive a message before the task to add
205 // the filter is run on the IO thread. 202 // the filter is run on the IO thread.
206 OnAddFilter(); 203 OnAddFilter();
207 204
205 // We cache off the peer_pid so it can be safely accessed from both threads.
206 peer_pid_ = channel_->peer_pid();
207 for (size_t i = 0; i < filters_.size(); ++i)
208 filters_[i]->OnChannelConnected(peer_pid);
209
208 // See above comment about using listener_task_runner_ here. 210 // See above comment about using listener_task_runner_ here.
209 listener_task_runner_->PostTask( 211 listener_task_runner_->PostTask(
210 FROM_HERE, base::Bind(&Context::OnDispatchConnected, this)); 212 FROM_HERE, base::Bind(&Context::OnDispatchConnected, this));
211 } 213 }
212 214
213 // Called on the IPC::Channel thread 215 // Called on the IPC::Channel thread
214 void ChannelProxy::Context::OnChannelError() { 216 void ChannelProxy::Context::OnChannelError() {
215 for (size_t i = 0; i < filters_.size(); ++i) 217 for (size_t i = 0; i < filters_.size(); ++i)
216 filters_[i]->OnChannelError(); 218 filters_[i]->OnChannelError();
217 219
(...skipping 16 matching lines...) Expand all
234 } 236 }
235 237
236 for (size_t i = 0; i < filters_.size(); ++i) 238 for (size_t i = 0; i < filters_.size(); ++i)
237 filters_[i]->OnFilterAdded(channel_.get()); 239 filters_[i]->OnFilterAdded(channel_.get());
238 } 240 }
239 241
240 // Called on the IPC::Channel thread 242 // Called on the IPC::Channel thread
241 void ChannelProxy::Context::OnChannelClosed() { 243 void ChannelProxy::Context::OnChannelClosed() {
242 // It's okay for IPC::ChannelProxy::Close to be called more than once, which 244 // It's okay for IPC::ChannelProxy::Close to be called more than once, which
243 // would result in this branch being taken. 245 // would result in this branch being taken.
244 if (!channel_) 246 if (!channel_.get())
245 return; 247 return;
246 248
247 for (size_t i = 0; i < filters_.size(); ++i) { 249 for (size_t i = 0; i < filters_.size(); ++i) {
248 filters_[i]->OnChannelClosing(); 250 filters_[i]->OnChannelClosing();
249 filters_[i]->OnFilterRemoved(); 251 filters_[i]->OnFilterRemoved();
250 } 252 }
251 253
252 // We don't need the filters anymore. 254 // We don't need the filters anymore.
253 message_filter_router_->Clear(); 255 message_filter_router_->Clear();
254 filters_.clear(); 256 filters_.clear();
255 257
256 channel_.reset(); 258 channel_.reset();
257 259
258 // Balance with the reference taken during startup. This may result in 260 // Balance with the reference taken during startup. This may result in
259 // self-destruction. 261 // self-destruction.
260 Release(); 262 Release();
261 } 263 }
262 264
263 void ChannelProxy::Context::Clear() { 265 void ChannelProxy::Context::Clear() {
264 listener_ = NULL; 266 listener_ = NULL;
265 } 267 }
266 268
267 // Called on the IPC::Channel thread 269 // Called on the IPC::Channel thread
268 void ChannelProxy::Context::OnSendMessage(scoped_ptr<Message> message) { 270 void ChannelProxy::Context::OnSendMessage(scoped_ptr<Message> message) {
269 if (!channel_) { 271 if (!channel_.get()) {
270 OnChannelClosed(); 272 OnChannelClosed();
271 return; 273 return;
272 } 274 }
273
274 if (!channel_->Send(message.release())) 275 if (!channel_->Send(message.release()))
275 OnChannelError(); 276 OnChannelError();
276 } 277 }
277 278
278 // Called on the IPC::Channel thread 279 // Called on the IPC::Channel thread
279 void ChannelProxy::Context::OnAddFilter() { 280 void ChannelProxy::Context::OnAddFilter() {
280 // Our OnChannelConnected method has not yet been called, so we can't be
281 // sure that channel_ is valid yet. When OnChannelConnected *is* called,
282 // it invokes OnAddFilter, so any pending filter(s) will be added at that
283 // time.
284 if (peer_pid_ == base::kNullProcessId)
285 return;
286
287 std::vector<scoped_refptr<MessageFilter> > new_filters; 281 std::vector<scoped_refptr<MessageFilter> > new_filters;
288 { 282 {
289 base::AutoLock auto_lock(pending_filters_lock_); 283 base::AutoLock auto_lock(pending_filters_lock_);
290 new_filters.swap(pending_filters_); 284 new_filters.swap(pending_filters_);
291 } 285 }
292 286
293 for (size_t i = 0; i < new_filters.size(); ++i) { 287 for (size_t i = 0; i < new_filters.size(); ++i) {
294 filters_.push_back(new_filters[i]); 288 filters_.push_back(new_filters[i]);
295 289
296 message_filter_router_->AddFilter(new_filters[i].get()); 290 message_filter_router_->AddFilter(new_filters[i].get());
297 291
298 // The channel has already been created and connected, so we need to 292 // If the channel has already been created, then we need to send this
299 // inform the filters right now. 293 // message so that the filter gets access to the Channel.
300 new_filters[i]->OnFilterAdded(channel_.get()); 294 if (channel_.get())
301 new_filters[i]->OnChannelConnected(peer_pid_); 295 new_filters[i]->OnFilterAdded(channel_.get());
296 // Ditto for if the channel has been connected.
297 if (peer_pid_)
298 new_filters[i]->OnChannelConnected(peer_pid_);
302 } 299 }
303 } 300 }
304 301
305 // Called on the IPC::Channel thread 302 // Called on the IPC::Channel thread
306 void ChannelProxy::Context::OnRemoveFilter(MessageFilter* filter) { 303 void ChannelProxy::Context::OnRemoveFilter(MessageFilter* filter) {
307 if (peer_pid_ == base::kNullProcessId) { 304 if (!channel_.get())
308 // The channel is not yet connected, so any filters are still pending.
309 base::AutoLock auto_lock(pending_filters_lock_);
310 for (size_t i = 0; i < pending_filters_.size(); ++i) {
311 if (pending_filters_[i].get() == filter) {
312 filter->OnFilterRemoved();
313 pending_filters_.erase(pending_filters_.begin() + i);
314 return;
315 }
316 }
317 return;
318 }
319 if (!channel_)
320 return; // The filters have already been deleted. 305 return; // The filters have already been deleted.
321 306
322 message_filter_router_->RemoveFilter(filter); 307 message_filter_router_->RemoveFilter(filter);
323 308
324 for (size_t i = 0; i < filters_.size(); ++i) { 309 for (size_t i = 0; i < filters_.size(); ++i) {
325 if (filters_[i].get() == filter) { 310 if (filters_[i].get() == filter) {
326 filter->OnFilterRemoved(); 311 filter->OnFilterRemoved();
327 filters_.erase(filters_.begin() + i); 312 filters_.erase(filters_.begin() + i);
328 return; 313 return;
329 } 314 }
(...skipping 197 matching lines...) Expand 10 before | Expand all | Expand 10 after
527 Channel* channel = context_.get()->channel_.get(); 512 Channel* channel = context_.get()->channel_.get();
528 // Channel must have been created first. 513 // Channel must have been created first.
529 DCHECK(channel) << context_.get()->channel_id_; 514 DCHECK(channel) << context_.get()->channel_id_;
530 return channel->GetPeerEuid(peer_euid); 515 return channel->GetPeerEuid(peer_euid);
531 } 516 }
532 #endif 517 #endif
533 518
534 //----------------------------------------------------------------------------- 519 //-----------------------------------------------------------------------------
535 520
536 } // namespace IPC 521 } // namespace IPC
OLDNEW
« no previous file with comments | « ipc/ipc_channel_proxy.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698