Issue 5706001: An exprimental fix for Bug 65940....

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 5706001: An exprimental fix for Bug 65940.... (Closed)

Created:
10 years ago by Hironori Bono

Modified:
8 years, 5 months ago

Reviewers:
dmac, cpu_(ooo_6.6-7.5)

CC:
chromium-reviews, Timur Iskhodzhanov, Alexander Potapenko, pam+watch_chromium.org, stuartmorgan+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src/

Visibility:
Public.

Description

An exprimental fix for Bug 65940. It seems these leaks are caused by a reference-count mismatch for a ChannelProxy::Context object. (This class expects to increase its reference count in OnChannelOpened() and to decrease it in OnChannelClosed(). On the other hand, we cannot call OnChannelClosed() after calling ClearIPCMessageLoop() because it detaches the message loop of a ChannelProxy::Context object. That is, calling ChannelProxy::Close() in ChannelProxy::~ChannelProxy() cannot decrease the reference count to its context.) This change just explicitly calls ChannelProxy::Close() before calling ChannelProxy::ClearIPCMessageLoop(). BUG=65940 TEST=make the valgrind bot green without suppressions. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=68831

Patch Set 1 #

Created: 10 years ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+5 lines, -19 lines)			Patch
	M	chrome/common/child_thread.cc	View		1 chunk	+5 lines, -0 lines	0 comments	Download
	M	tools/valgrind/memcheck/suppressions.txt	View		1 chunk	+0 lines, -19 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Hironori Bono

Greetings, Even though I'm not sure if this is really a good fix, it fixes ...

10 years ago (2010-12-09 10:26:30 UTC) #1

jam

8 years, 5 months ago (2012-06-29 20:55:34 UTC) #3

I just saw this change (a little delayed, I know :) ). This makes the
channel_->ClearIPCMessageLoop(); call in the next line useless, which makes it
harder to watch for process shutdown in the browser (before we used to wait for
the channel to close to know that the process was shutdown). As a result, we had
to add extra complexity to also wait on it in r101435.

I'd rather have a suppression for valgrind than extra complexity in the shipping
code.

+cpu

Expand Messages | Collapse Messages