New issue
Advanced search Search tips

Issue 805424 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

NetworkChangeNotifierAutoDetect.vpnAccessible() triggers Android socket leak

Project Member Reported by pauljensen@chromium.org, Jan 24 2018

Issue description

Current this function calls:
             try {
                network.getSocketFactory().createSocket().close();
             } catch (IOException e) {
But the Android createSocket() function leaks when bindSocket() throws:
http://androidxref.com/5.0.0_r2/xref/frameworks/base/core/java/android/net/Network.java#178
        public Socket createSocket() throws IOException {
            Socket socket = new Socket();
            bindSocket(socket);
            return socket;
        }


Internal bug b/72124526
 
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 24 2018

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

commit 8bab3d3e45fa7dda5cc2efd49e0098aa04bd1634
Author: Paul Jensen <pauljensen@chromium.org>
Date: Wed Jan 24 16:58:14 2018

Avoid using Network.getSocketFactory().createSocket() because it leaks.

Bug:  805424 
Change-Id: I11d43256ba552030e3d8d6a1907f590eae6a66a2
Reviewed-on: https://chromium-review.googlesource.com/883183
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531566}
[modify] https://crrev.com/8bab3d3e45fa7dda5cc2efd49e0098aa04bd1634/net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java
[modify] https://crrev.com/8bab3d3e45fa7dda5cc2efd49e0098aa04bd1634/net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java

Labels: Merge-Request-65
Project Member

Comment 4 by sheriffbot@chromium.org, Jan 27 2018

Labels: -Merge-Request-65 Hotlist-Merge-Approved Merge-Approved-65
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 29 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9170422e576a6f9b881196e3b892797b622369e1

commit 9170422e576a6f9b881196e3b892797b622369e1
Author: Paul Jensen <pauljensen@chromium.org>
Date: Mon Jan 29 12:26:07 2018

Avoid using Network.getSocketFactory().createSocket() because it leaks.

TBR=pauljensen@chromium.org

(cherry picked from commit 8bab3d3e45fa7dda5cc2efd49e0098aa04bd1634)

Bug:  805424 
Change-Id: I11d43256ba552030e3d8d6a1907f590eae6a66a2
Reviewed-on: https://chromium-review.googlesource.com/883183
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Paul Jensen <pauljensen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#531566}
Reviewed-on: https://chromium-review.googlesource.com/889804
Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#133}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/9170422e576a6f9b881196e3b892797b622369e1/net/android/java/src/org/chromium/net/NetworkChangeNotifierAutoDetect.java
[modify] https://crrev.com/9170422e576a6f9b881196e3b892797b622369e1/net/android/javatests/src/org/chromium/net/NetworkChangeNotifierTest.java

Status: Fixed (was: Started)

Sign in to add a comment