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

Issue 10855013: Ensure we don't have zombie child processes that get started but never get a chance to connect to t… (Closed)

Created:
8 years, 4 months ago by jam
Modified:
8 years, 4 months ago
Reviewers:
Avi (use Gerrit)
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam
Visibility:
Public.

Description

Ensure we don't have zombie child processes that get started but never get a chance to connect to the browser process. Since they never connected, they will never have an OnChannelError and so they don't get destroyed. This was causing the sharding_supervisor script to hang on Linux since there were child processes that didn't exit. It looks like there are race conditions in some tests where child processes start but the browser closes before they can connect. Probably we only see this on Linux because it's faster than other platforms or the shutdown timing is different. BUG=140054 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=150278

Patch Set 1 #

Patch Set 2 : #

Total comments: 4

Patch Set 3 : #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -16 lines) Patch
M content/common/child_thread.h View 4 chunks +13 lines, -10 lines 0 comments Download
M content/common/child_thread.cc View 1 2 8 chunks +23 lines, -6 lines 2 comments Download

Messages

Total messages: 4 (0 generated)
Avi (use Gerrit)
http://codereview.chromium.org/10855013/diff/3001/content/common/child_thread.cc File content/common/child_thread.cc (right): http://codereview.chromium.org/10855013/diff/3001/content/common/child_thread.cc#newcode42 content/common/child_thread.cc:42: // terminates child processes automatically. For unsanboxed processes (i.e. ...
8 years, 4 months ago (2012-08-07 04:50:51 UTC) #1
jam
http://codereview.chromium.org/10855013/diff/3001/content/common/child_thread.cc File content/common/child_thread.cc (right): http://codereview.chromium.org/10855013/diff/3001/content/common/child_thread.cc#newcode42 content/common/child_thread.cc:42: // terminates child processes automatically. For unsanboxed processes (i.e. ...
8 years, 4 months ago (2012-08-07 04:54:27 UTC) #2
Avi (use Gerrit)
lgtm http://codereview.chromium.org/10855013/diff/6003/content/common/child_thread.cc File content/common/child_thread.cc (right): http://codereview.chromium.org/10855013/diff/6003/content/common/child_thread.cc#newcode43 content/common/child_thread.cc:43: // plugins), PluginThread has EnsureTerminateMessageFilter. EnsureTerminateMessageFilter? So we've ...
8 years, 4 months ago (2012-08-07 04:57:48 UTC) #3
jam
8 years, 4 months ago (2012-08-07 05:07:46 UTC) #4
http://codereview.chromium.org/10855013/diff/6003/content/common/child_thread.cc
File content/common/child_thread.cc (right):

http://codereview.chromium.org/10855013/diff/6003/content/common/child_thread...
content/common/child_thread.cc:43: // plugins), PluginThread has
EnsureTerminateMessageFilter.
On 2012/08/07 04:57:48, Avi wrote:
> EnsureTerminateMessageFilter? So we've seen this before? :) Every time we fix
an
> issue with plugins we should check to see if the misbehavedness of them
extends
> to a lesser extent to other things.

What happened is plugins got fixed for all platforms initially (since they're
not sandboxed and so the sandbox' job object doesn't quit them). workers/utility
processes quit automatically on windows because of the job object. but on linux,
that isn't the case, so this filter got added only for renderers. i saw this
issue for workers as well, so i made it happen for all child processes on linux
in another change last week.

Powered by Google App Engine
This is Rietveld 408576698