Commit f775d108 authored by alexeypa@chromium.org's avatar alexeypa@chromium.org

Marked IPC::ChannelProxy as non thread-safe. Added DCHECKs to verify that...

Marked IPC::ChannelProxy as non thread-safe. Added DCHECKs to verify that public methods (with Send() being the only exception) are called on the thread that created the object. 

Tests calling Send() from a wrong thread should be fixed first before similar check can be added to Send(). See http://crbug.com/163523 for details.

BUG=163091,163523

Review URL: https://chromiumcodereview.appspot.com/11308278

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@171106 0039d316-1c4b-4281-b951-d872f2087c98
parent 14649ecd
...@@ -301,12 +301,15 @@ ChannelProxy::ChannelProxy(Context* context) ...@@ -301,12 +301,15 @@ ChannelProxy::ChannelProxy(Context* context)
} }
ChannelProxy::~ChannelProxy() { ChannelProxy::~ChannelProxy() {
DCHECK(CalledOnValidThread());
Close(); Close();
} }
void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle, void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle,
Channel::Mode mode, Channel::Mode mode,
bool create_pipe_now) { bool create_pipe_now) {
DCHECK(CalledOnValidThread());
DCHECK(!did_init_); DCHECK(!did_init_);
#if defined(OS_POSIX) #if defined(OS_POSIX)
// When we are creating a server on POSIX, we need its file descriptor // When we are creating a server on POSIX, we need its file descriptor
...@@ -338,6 +341,8 @@ void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle, ...@@ -338,6 +341,8 @@ void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle,
} }
void ChannelProxy::Close() { void ChannelProxy::Close() {
DCHECK(CalledOnValidThread());
// Clear the backpointer to the listener so that any pending calls to // Clear the backpointer to the listener so that any pending calls to
// Context::OnDispatchMessage or OnDispatchError will be ignored. It is // Context::OnDispatchMessage or OnDispatchError will be ignored. It is
// possible that the channel could be closed while it is receiving messages! // possible that the channel could be closed while it is receiving messages!
...@@ -351,6 +356,9 @@ void ChannelProxy::Close() { ...@@ -351,6 +356,9 @@ void ChannelProxy::Close() {
bool ChannelProxy::Send(Message* message) { bool ChannelProxy::Send(Message* message) {
DCHECK(did_init_); DCHECK(did_init_);
// TODO(alexeypa): add DCHECK(CalledOnValidThread()) here. Currently there are
// tests that call Send() from a wrong thread. See http://crbug.com/163523.
if (outgoing_message_filter()) if (outgoing_message_filter())
message = outgoing_message_filter()->Rewrite(message); message = outgoing_message_filter()->Rewrite(message);
...@@ -366,16 +374,22 @@ bool ChannelProxy::Send(Message* message) { ...@@ -366,16 +374,22 @@ bool ChannelProxy::Send(Message* message) {
} }
void ChannelProxy::AddFilter(MessageFilter* filter) { void ChannelProxy::AddFilter(MessageFilter* filter) {
DCHECK(CalledOnValidThread());
context_->AddFilter(filter); context_->AddFilter(filter);
} }
void ChannelProxy::RemoveFilter(MessageFilter* filter) { void ChannelProxy::RemoveFilter(MessageFilter* filter) {
DCHECK(CalledOnValidThread());
context_->ipc_task_runner()->PostTask( context_->ipc_task_runner()->PostTask(
FROM_HERE, base::Bind(&Context::OnRemoveFilter, context_.get(), FROM_HERE, base::Bind(&Context::OnRemoveFilter, context_.get(),
make_scoped_refptr(filter))); make_scoped_refptr(filter)));
} }
void ChannelProxy::ClearIPCTaskRunner() { void ChannelProxy::ClearIPCTaskRunner() {
DCHECK(CalledOnValidThread());
context()->ClearIPCTaskRunner(); context()->ClearIPCTaskRunner();
} }
...@@ -383,6 +397,8 @@ void ChannelProxy::ClearIPCTaskRunner() { ...@@ -383,6 +397,8 @@ void ChannelProxy::ClearIPCTaskRunner() {
// See the TODO regarding lazy initialization of the channel in // See the TODO regarding lazy initialization of the channel in
// ChannelProxy::Init(). // ChannelProxy::Init().
int ChannelProxy::GetClientFileDescriptor() { int ChannelProxy::GetClientFileDescriptor() {
DCHECK(CalledOnValidThread());
Channel* channel = context_.get()->channel_.get(); Channel* channel = context_.get()->channel_.get();
// Channel must have been created first. // Channel must have been created first.
DCHECK(channel) << context_.get()->channel_id_; DCHECK(channel) << context_.get()->channel_id_;
...@@ -390,6 +406,8 @@ int ChannelProxy::GetClientFileDescriptor() { ...@@ -390,6 +406,8 @@ int ChannelProxy::GetClientFileDescriptor() {
} }
int ChannelProxy::TakeClientFileDescriptor() { int ChannelProxy::TakeClientFileDescriptor() {
DCHECK(CalledOnValidThread());
Channel* channel = context_.get()->channel_.get(); Channel* channel = context_.get()->channel_.get();
// Channel must have been created first. // Channel must have been created first.
DCHECK(channel) << context_.get()->channel_id_; DCHECK(channel) << context_.get()->channel_id_;
...@@ -397,6 +415,8 @@ int ChannelProxy::TakeClientFileDescriptor() { ...@@ -397,6 +415,8 @@ int ChannelProxy::TakeClientFileDescriptor() {
} }
bool ChannelProxy::GetClientEuid(uid_t* client_euid) const { bool ChannelProxy::GetClientEuid(uid_t* client_euid) const {
DCHECK(CalledOnValidThread());
Channel* channel = context_.get()->channel_.get(); Channel* channel = context_.get()->channel_.get();
// Channel must have been created first. // Channel must have been created first.
DCHECK(channel) << context_.get()->channel_id_; DCHECK(channel) << context_.get()->channel_id_;
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "base/synchronization/lock.h" #include "base/synchronization/lock.h"
#include "base/threading/non_thread_safe.h"
#include "ipc/ipc_channel.h" #include "ipc/ipc_channel.h"
#include "ipc/ipc_channel_handle.h" #include "ipc/ipc_channel_handle.h"
#include "ipc/ipc_listener.h" #include "ipc/ipc_listener.h"
...@@ -51,7 +52,7 @@ class SendCallbackHelper; ...@@ -51,7 +52,7 @@ class SendCallbackHelper;
// The consumer of IPC::ChannelProxy is responsible for allocating the Thread // The consumer of IPC::ChannelProxy is responsible for allocating the Thread
// instance where the IPC::Channel will be created and operated. // instance where the IPC::Channel will be created and operated.
// //
class IPC_EXPORT ChannelProxy : public Sender { class IPC_EXPORT ChannelProxy : public Sender, public base::NonThreadSafe {
public: public:
struct MessageFilterTraits; struct MessageFilterTraits;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment