Fix asan test failure in P2PSocketClient::Init. Make sure P2PSocketClient::Init only access |deleg…

content/renderer/p2p/socket_client.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/p2p/socket_client.h"
 
 #include "base/bind.h"
 #include "base/message_loop_proxy.h"
 #include "content/common/p2p_messages.h"
 #include "content/renderer/p2p/socket_dispatcher.h"
@@ skipping 10 matching lines @@
 
 P2PSocketClient::~P2PSocketClient() {
   DCHECK(state_ == STATE_CLOSED || state_ == STATE_UNINITIALIZED);
 }
 
 void P2PSocketClient::Init(
     P2PSocketType type,
     const net::IPEndPoint& local_address,
     const net::IPEndPoint& remote_address,
     P2PSocketClient::Delegate* delegate) {
+  DCHECK(delegate_message_loop_->BelongsToCurrentThread());

[tommi (sloooow) - chröme, 2012/09/20 11:48:44]

if the delegate and ipc threads are not always the same, then this DCHECK will
fire because you post a task to this function from the ipc thread below.

+  // |delegate_| is only accessesed on |delegate_message_loop_|.
+  delegate_ = delegate;

[tommi (sloooow) - chröme, 2012/09/20 11:48:44]

...and since you post that task from the ipc thread, you end up touching this
variable from both threads always. Hence, I don't think you've fixed the
problem.

[perkj_chrome, 2012/09/20 12:22:32]

Humm- Stupid me. Sorry - should not have sent out for review before testintg and
reviewing myself.this is still the reason.. 

+
   if (!ipc_message_loop_->BelongsToCurrentThread()) {

[tommi (sloooow) - chröme, 2012/09/20 11:48:44]

btw, maybe you could add a note here (assuming that it's true), that
ipc_message_loop_ and delegate_message_loop_ can be the same loop?

[perkj_chrome, 2012/09/20 12:22:32]

In chrome now, they are not the same. 
But I don't think this class should have to know that or care. I remove the if
statement. 


On 2012/09/20 11:48:44, tommi wrote:

     ipc_message_loop_->PostTask(
         FROM_HERE, base::Bind(&P2PSocketClient::Init, this, type, local_address,
                               remote_address, delegate));

[tommi (sloooow) - chröme, 2012/09/20 11:48:44]

in order to not use the |delegate| parameter on the other thread, can you set it
to NULL here and add documentation for why?

[perkj_chrome, 2012/09/20 12:22:32]

Done.

     return;
   }
 
   DCHECK_EQ(state_, STATE_UNINITIALIZED);

[tommi (sloooow) - chröme, 2012/09/20 11:48:44]

and then here, you can DCHECK that delegate_ is not NULL :)

[perkj_chrome, 2012/09/20 12:22:32]

Done.

   state_ = STATE_OPENING;
-  delegate_ = delegate;
   socket_id_ = dispatcher_->RegisterClient(this);
   dispatcher_->SendP2PMessage(new P2PHostMsg_CreateSocket(
       type, socket_id_, local_address, remote_address));
 }
 
 void P2PSocketClient::Send(const net::IPEndPoint& address,
                            const std::vector<char>& data) {
   if (!ipc_message_loop_->BelongsToCurrentThread()) {
     ipc_message_loop_->PostTask(
         FROM_HERE, base::Bind(&P2PSocketClient::Send, this, address, data));
@@ skipping 10 matching lines @@
 void P2PSocketClient::Close() {
   DCHECK(delegate_message_loop_->BelongsToCurrentThread());
 
   delegate_ = NULL;
 
   ipc_message_loop_->PostTask(
       FROM_HERE, base::Bind(&P2PSocketClient::DoClose, this));
 }
 
 void P2PSocketClient::DoClose() {
+  DCHECK(ipc_message_loop_->BelongsToCurrentThread());
   if (dispatcher_) {
     if (state_ == STATE_OPEN || state_ == STATE_OPENING ||
         state_ == STATE_ERROR) {
       dispatcher_->SendP2PMessage(new P2PHostMsg_DestroySocket(socket_id_));
     }
     dispatcher_->UnregisterClient(socket_id_);
   }
 
   state_ = STATE_CLOSED;
 }
 
 void P2PSocketClient::set_delegate(Delegate* delegate) {
   DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   delegate_ = delegate;
 }
 
 void P2PSocketClient::OnSocketCreated(const net::IPEndPoint& address) {
   DCHECK(ipc_message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STATE_OPENING);
   state_ = STATE_OPEN;
 
   delegate_message_loop_->PostTask(
       FROM_HERE,
       base::Bind(&P2PSocketClient::DeliverOnSocketCreated, this, address));
 }
 
 void P2PSocketClient::DeliverOnSocketCreated(const net::IPEndPoint& address) {
+  DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   if (delegate_)
     delegate_->OnOpen(address);
 }
 
 void P2PSocketClient::OnIncomingTcpConnection(const net::IPEndPoint& address) {
   DCHECK(ipc_message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STATE_OPEN);
 
   scoped_refptr<P2PSocketClient> new_client = new P2PSocketClient(dispatcher_);
   new_client->socket_id_ = dispatcher_->RegisterClient(new_client);
   new_client->state_ = STATE_OPEN;
   new_client->delegate_message_loop_ = delegate_message_loop_;
 
   dispatcher_->SendP2PMessage(new P2PHostMsg_AcceptIncomingTcpConnection(
       socket_id_, address, new_client->socket_id_));
 
   delegate_message_loop_->PostTask(
       FROM_HERE, base::Bind(&P2PSocketClient::DeliverOnIncomingTcpConnection,
                             this, address, new_client));
 }
 
 void P2PSocketClient::DeliverOnIncomingTcpConnection(
     const net::IPEndPoint& address, scoped_refptr<P2PSocketClient> new_client) {
+  DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   if (delegate_)
     delegate_->OnIncomingTcpConnection(address, new_client);
 }
 
 void P2PSocketClient::OnError() {
   DCHECK(ipc_message_loop_->BelongsToCurrentThread());
   state_ = STATE_ERROR;
 
   delegate_message_loop_->PostTask(
       FROM_HERE, base::Bind(&P2PSocketClient::DeliverOnError, this));
 }
 
 void P2PSocketClient::DeliverOnError() {
+  DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   if (delegate_)
     delegate_->OnError();
 }
 
 void P2PSocketClient::OnDataReceived(const net::IPEndPoint& address,
                                      const std::vector<char>& data) {
   DCHECK(ipc_message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(STATE_OPEN, state_);
   delegate_message_loop_->PostTask(
       FROM_HERE,
       base::Bind(&P2PSocketClient::DeliverOnDataReceived, this, address, data));
 }
 
 void P2PSocketClient::DeliverOnDataReceived(const net::IPEndPoint& address,
                                             const std::vector<char>& data) {
+  DCHECK(delegate_message_loop_->BelongsToCurrentThread());
   if (delegate_)
     delegate_->OnDataReceived(address, data);
 }
 
 void P2PSocketClient::Detach() {
   DCHECK(ipc_message_loop_->BelongsToCurrentThread());
   dispatcher_ = NULL;
   OnError();
 }
 
 }  // namespace content