Fix up uninitialized member variable

Fix up problem with uninitialized member variable.

BUG=none
TEST=build

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=69341

[dmac, 2010-12-15 20:25:56]

David, do you mind taking a quick look? Running it through the trybots now.

[jam, 2010-12-15 20:35:40]

hi, per http://dev.chromium.org/developers/committers-responsibility, please
send changes to someone who's familiar with this change.

which test is failing?  is there no way for it to delete the reply_msg so that
we don't have to modify the base IPC code to work around memory bots for a test?

[dmaclach1, 2010-12-15 20:41:39]

On Wed, Dec 15, 2010 at 12:35 PM,  <jam@chromium.org> wrote:
> hi, per http://dev.chromium.org/developers/committers-responsibility, please
> send changes to someone who's familiar with this change.
>
> which test is failing?  is there no way for it to delete the reply_msg so
> that
> we don't have to modify the base IPC code to work around memory bots for a
> test?
>
> http://codereview.chromium.org/5836002/
>

Sorry Jam. dhollowa was familiar with the failure, so that's why I
sent him the review. The code in question needs to clean up after
itself when the exception is triggered. It created an object, but
didn't take ownership of it properly.

[dhollowa, 2010-12-15 21:19:45]

http://build.chromium.org/p/chromium.memory/builders/Linux%20Heapcheck/builds...


Leak of 256 bytes in 4 objects allocated from:
	@ 4be9a5 Pickle::Resize
	@ 4bed51 Pickle
	@ 468507 Message
	@ 4753e7 IPC::SyncMessage::GenerateReply
	@ 4288d9 IPC::MessageWithReply::DispatchDelayReply
	@ 428d9d ::Worker::OnMessageReceived
	@ 45fb08 IPC::ChannelProxy::Context::OnDispatchMessage
	@ 473d78 IPC::SyncChannel::ReceivedSyncMsgQueue::DispatchMessages
	@ 46ca56 IPC::SyncChannel::SyncContext::DispatchMessages
	@ 46cb6a IPC::SyncChannel::OnWaitableEventSignaled
	@ 50baa9 base::AsyncCallbackTask::Run
	@ 4a3355 MessageLoop::RunTask
	@ 4a3446 MessageLoop::DeferOrRunPendingTask
	@ 4a370c MessageLoop::DoWork
	@ 4ae7eb base::MessagePumpDefault::Run
	@ 4a4036 MessageLoop::RunInternal
	@ 4a406d MessageLoop::RunHandler
	@ 4a4172 MessageLoop::Run
	@ 46c3bd IPC::SyncChannel::WaitForReplyWithNestedMessageLoop
	@ 46cc41 IPC::SyncChannel::WaitForReply
	@ 46d170 IPC::SyncChannel::SendWithTimeout
	@ 46bcc0 IPC::SyncChannel::Send
	@ 4282f9 ::Worker::Send
	@ 42edb4 ::Worker::SendDouble
	@ 42efed ::RecursiveServer::Run
	@ 42d097 ::Worker::OnStart
	@ 425b65 DispatchToMethod
	@ 425bc2 RunnableMethod::Run
	@ 4a3355 MessageLoop::RunTask
	@ 4a3446 MessageLoop::DeferOrRunPendingTask
	@ 4a370c MessageLoop::DoWork
	@ 4ae7eb base::MessagePumpDefault::Run

[dhollowa, 2010-12-15 21:21:44]

On 2010/12/15 20:25:56, dmac wrote:
> David, do you mind taking a quick look? Running it through the trybots now.

Dave, the trys failed.  You'll probably need a newer revision number on your try
command (--revision 123).

[jam, 2010-12-15 21:23:16]

On Wed, Dec 15, 2010 at 12:41 PM, David Maclachlan <dmaclach@google.com>wrote:

> On Wed, Dec 15, 2010 at 12:35 PM,  <jam@chromium.org> wrote:
> > hi, per http://dev.chromium.org/developers/committers-responsibility,
> please
> > send changes to someone who's familiar with this change.
> >
> > which test is failing?  is there no way for it to delete the reply_msg so
> > that
> > we don't have to modify the base IPC code to work around memory bots for
> a
> > test?
> >
> > http://codereview.chromium.org/5836002/
> >
>
> Sorry Jam. dhollowa was familiar with the failure, so that's why I
> sent him the review. The code in question needs to clean up after
> itself when the exception is triggered. It created an object, but
> didn't take ownership of it properly.
>

hmm I still don't understand this change.  so in both conditions of the if
statement reply.release() is called.  which path does is this attempting to
fix?

[dhollowa, 2010-12-15 21:30:44]

Also,

http://build.chromium.org/p/chromium.memory/builders/Linux%20Tests%20(valgrin...

and

http://build.chromium.org/p/chromium.memory/builders/Chromium%20Mac%20(valgri...

That look similar to:

Conditional jump or move depends on uninitialised value(s)
  IPC::Channel::ChannelImpl::Close()
(uilder/build/src/ipc/ipc_channel_posix.cc:1087)
  IPC::Channel::ChannelImpl::ClosePipeOnError()
(uilder/build/src/ipc/ipc_channel_posix.cc:1046)
  IPC::Channel::ChannelImpl::OnFileCanReadWithoutBlocking(int)
(uilder/build/src/ipc/ipc_channel_posix.cc:997)
 
base::MessagePumpLibevent::FileDescriptorWatcher::OnFileCanReadWithoutBlocking(int,
base::MessagePumpLibevent*) (uilder/build/src/base/message_pump_libevent.cc:96)
  base::MessagePumpLibevent::OnLibeventNotification(int, short, void*)
(uilder/build/src/base/message_pump_libevent.cc:248)
  event_process_active (uilder/build/src/third_party/libevent/event.c:385)
  event_base_loop (uilder/build/src/third_party/libevent/event.c:525)
  base::MessagePumpLibevent::Run(base::MessagePump::Delegate*)
(uilder/build/src/base/message_pump_libevent.cc:301)
  MessageLoop::RunInternal() (uilder/build/src/base/message_loop.cc:271)
  MessageLoop::RunHandler() (uilder/build/src/base/message_loop.cc:243)
  MessageLoop::Run() (uilder/build/src/base/message_loop.cc:221)
  base::Thread::Run(MessageLoop*) (uilder/build/src/base/thread.cc:140)
  base::Thread::ThreadMain() (uilder/build/src/base/thread.cc:164)
  ThreadFunc(void*) (uilder/build/src/base/platform_thread_posix.cc:53)
  start_thread (/lib/tls/i686/cmov/libpthread-2.7.so)

[jam, 2010-12-15 21:58:37]

On Wed, Dec 15, 2010 at 1:23 PM, John Abd-El-Malek <jam@chromium.org> wrote:

>
>
> On Wed, Dec 15, 2010 at 12:41 PM, David Maclachlan <dmaclach@google.com>wrote:
>
>> On Wed, Dec 15, 2010 at 12:35 PM,  <jam@chromium.org> wrote:
>> > hi, per http://dev.chromium.org/developers/committers-responsibility,
>> please
>> > send changes to someone who's familiar with this change.
>> >
>> > which test is failing?  is there no way for it to delete the reply_msg
>> so
>> > that
>> > we don't have to modify the base IPC code to work around memory bots for
>> a
>> > test?
>> >
>> > http://codereview.chromium.org/5836002/
>> >
>>
>> Sorry Jam. dhollowa was familiar with the failure, so that's why I
>> sent him the review. The code in question needs to clean up after
>> itself when the exception is triggered.
>
>
is this a C++ exception being thrown?  if so, where is this happening?
 that's against the coding guidelines.


> It created an object, but
>> didn't take ownership of it properly.
>>
>
> hmm I still don't understand this change.  so in both conditions of the if
> statement reply.release() is called.  which path does is this attempting to
> fix?
>

[dmac, 2010-12-15 22:56:02]

On 2010/12/15 21:58:37, John Abd-El-Malek wrote:
> On Wed, Dec 15, 2010 at 1:23 PM, John Abd-El-Malek <mailto:jam@chromium.org>
wrote:
> 
> >
> >
> > On Wed, Dec 15, 2010 at 12:41 PM, David Maclachlan
<dmaclach@google.com>wrote:
> >
> >> On Wed, Dec 15, 2010 at 12:35 PM,  <mailto:jam@chromium.org> wrote:
> >> > hi, per http://dev.chromium.org/developers/committers-responsibility,
> >> please
> >> > send changes to someone who's familiar with this change.
> >> >
> >> > which test is failing?  is there no way for it to delete the reply_msg
> >> so
> >> > that
> >> > we don't have to modify the base IPC code to work around memory bots for
> >> a
> >> > test?
> >> >
> >> > http://codereview.chromium.org/5836002/
> >> >
> >>
> >> Sorry Jam. dhollowa was familiar with the failure, so that's why I
> >> sent him the review. The code in question needs to clean up after
> >> itself when the exception is triggered.
> >
> >
> is this a C++ exception being thrown?  if so, where is this happening?
>  that's against the coding guidelines.
> 
> 
> > It created an object, but
> >> didn't take ownership of it properly.
> >>
> >
> > hmm I still don't understand this change.  so in both conditions of the if
> > statement reply.release() is called.  which path does is this attempting to
> > fix?
> >

Jam, I apologize. Looking at it in detail, I realize I was wrong. For some
reason I thought DCHECK threw an exception, but that appears not to be the case.
I am going to continue looking into it. In the meantime, this fix will get rid
of some of the other valgrind complaints, and may cut down on some test
flakiness.

[jam, 2010-12-15 23:04:54]

lgtm

On Wed, Dec 15, 2010 at 2:56 PM, <dmaclach@chromium.org> wrote:

> On 2010/12/15 21:58:37, John Abd-El-Malek wrote:
>
>> On Wed, Dec 15, 2010 at 1:23 PM, John Abd-El-Malek <mailto:
>> jam@chromium.org>
>>
> wrote:
>
>  >
>> >
>> > On Wed, Dec 15, 2010 at 12:41 PM, David Maclachlan
>>
> <dmaclach@google.com>wrote:
>
>> >
>> >> On Wed, Dec 15, 2010 at 12:35 PM,  <mailto:jam@chromium.org> wrote:
>> >> > hi, per http://dev.chromium.org/developers/committers-responsibility
>> ,
>> >> please
>> >> > send changes to someone who's familiar with this change.
>> >> >
>> >> > which test is failing?  is there no way for it to delete the
>> reply_msg
>> >> so
>> >> > that
>> >> > we don't have to modify the base IPC code to work around memory bots
>> for
>> >> a
>> >> > test?
>> >> >
>> >> > http://codereview.chromium.org/5836002/
>> >> >
>> >>
>> >> Sorry Jam. dhollowa was familiar with the failure, so that's why I
>> >> sent him the review. The code in question needs to clean up after
>> >> itself when the exception is triggered.
>> >
>> >
>> is this a C++ exception being thrown?  if so, where is this happening?
>>  that's against the coding guidelines.
>>
>
>
>  > It created an object, but
>> >> didn't take ownership of it properly.
>> >>
>> >
>> > hmm I still don't understand this change.  so in both conditions of the
>> if
>> > statement reply.release() is called.  which path does is this attempting
>> to
>> > fix?
>> >
>>
>
> Jam, I apologize. Looking at it in detail, I realize I was wrong. For some
> reason I thought DCHECK threw an exception, but that appears not to be the
> case.
> I am going to continue looking into it. In the meantime, this fix will get
> rid
> of some of the other valgrind complaints, and may cut down on some test
> flakiness.
>
>
> http://codereview.chromium.org/5836002/
>