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

Issue 788068 link

Starred by 1 user
Status: Fixed
Owner:
█ yzshen@chromium.org
Last visit > 30 days ago
Closed: Nov 2017
Cc:
█ vamshi.k...@techmahindra.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

mojo channel fd leaks

Reported by zk562.st...@sina.com, Nov 23 2017 
UserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36

Steps to reproduce the problem:
1. Server side creates mojo channel which contains a listened handle;
2. Client side connects the mojo channel;
3. Server side accept connection and handle status changed into accepted;
4. The new accept fd will be moved into listened fd;
5. The listened fd will be leaked.

What is the expected behavior?
close the listened fd before moving operation.

What went wrong?
listened fd leaked

mojo/edk/system/channel_posix.cc
OnFileCanReadWithoutBlocking() {
  handle_ = std::move(accept_fd);
}

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 62.0.3202.94  Channel: stable
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: 

add reset into ScopedPlatformHandle& operator=(ScopedPlatformHandle&& other)

Comment 1 by vamshi.k...@techmahindra.com, Nov 27 2017

Cc: vamshi.k...@techmahindra.com
Components: Internals>Mojo
Labels: Triaged-ET TE-Needs-TriageHelp Needs-Triage-M62
This issue seems to be out of scope of triaging as it is related to mojo channel, hence adding TE-Needs-TriageHelp for further investigation of the issue.

Comment 2 by yzshen@chromium.org, Nov 30 2017

Owner: yzshen@chromium.org
Status: Assigned (was: Unconfirmed)
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 30 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0cdbf3eeab5ca5a9f387b7f2859a091c06e5b6c0

commit 0cdbf3eeab5ca5a9f387b7f2859a091c06e5b6c0
Author: Yuzhu Shen <yzshen@chromium.org>
Date: Thu Nov 30 23:34:28 2017

Fix assignment operator of ScopedPlatformHandle.

The previous code leaks the original handle.

BUG= 788068 

Change-Id: If6674b1b72b3a4b3e065e541c8ab2f4f20318298
Reviewed-on: https://chromium-review.googlesource.com/801234
Commit-Queue: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520745}
[modify] https://crrev.com/0cdbf3eeab5ca5a9f387b7f2859a091c06e5b6c0/mojo/edk/embedder/scoped_platform_handle.h

Comment 4 by yzshen@chromium.org, Nov 30 2017

Status: Fixed (was: Assigned)
Thanks a lot for the report!

Comment 5 by w...@chromium.org, Dec 1 2017

Labels: -Pri-2 M-64 Pri-1
We should merge this to M64; as part of  issue 753541  we just landed a change to make greater use of ScopedPlatformHandle, where previously we used raw PlatformHandles, so I expect this may have more impact now.

Comment 6 by yzshen@chromium.org, Dec 1 2017 

It seems we haven't branch M64. So if the change you mentioned in #5 doesn't get merged to earlier versions, we should be fine.

Thanks for the context information!
►

Sign in to add a comment