New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 597287 link

Starred by 8 users
Status: Fixed
Owner:
█ roc...@chromium.org
please use my google.com address
Closed: Mar 2016
Cc:
gov...@chromium.org
█ keta...@chromium.org
█ tinazh@chromium.org
█ dimu@chromium.org
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 0
Type: Bug



Sign in to add a comment

Child shell connection leaks FDs in M50

Project Member Reported by roc...@chromium.org, Mar 23 2016 
Registering a child with the shell allocates a new socket pair
and stashes one end (unscoped!) in the process host user data.

In a normal configuration (i.e. not Mash) we never pass it onto
the child to complete initialization. Thus the child end of the
pipe leaks and the parent end waits indefinitely for the child
to do something. The result is that every render process launch
leaks two FDs.

Comment 1 by roc...@chromium.org, Mar 23 2016

Labels: Merge-Request-50
Not really a merge request I guess?

To persist the context here, I want to land https://codereview.chromium.org/1824573006 directly on branch 2661 to fix this bug.

The bug is not present in any other branch, and landing this CL is much safer than trying to merge the set of changes which eliminated the bug on the master branch.

Comment 2 by tin...@google.com, Mar 23 2016

Labels: -Merge-Request-50 Merge-Approved-50
Chatted with Ketakid@ and rockot@, CrOS needs the fix asap and will trigger a trial build immediately to mitigate the risk. Approved for M50 (branch 2661).
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 23 2016

Labels: -merge-approved-50 merge-merged-2661
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b58a7118b948076fb9c9e2f11c1dbd123808160

commit 2b58a7118b948076fb9c9e2f11c1dbd123808160
Author: Ken Rockot <rockot@chromium.org>
Date: Wed Mar 23 17:35:27 2016

Don't attempt to register render processes with the shell

This CL simply deletes the code which initiates the
child-to-shell registration process, as it serves no practial
purpose in M50. The process has already been made much less
terrible since M50, so it is neither possible nor necessary
to land this change further upstream.

BUG= 597287 
R=jam@chromium.org

Review URL: https://codereview.chromium.org/1824573006 .

Cr-Commit-Position: refs/branch-heads/2661@{#359}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/2b58a7118b948076fb9c9e2f11c1dbd123808160/content/browser/renderer_host/render_process_host_impl.cc

Comment 4 by roc...@chromium.org, Mar 23 2016

Status: Fixed (was: Started)

Comment 5 by tin...@google.com, Mar 23 2016

Cc: dimu@chromium.org gov...@chromium.org
►

Sign in to add a comment