From 70b3f2aa7f381fc5caa4e6e02c0b479458b06411 Mon Sep 17 00:00:00 2001 From: xiaenlong Date: Fri, 2 Jun 2017 09:03:46 -0700 Subject: [PATCH] =?UTF-8?q?Fix=20`InspectorPackagerConnection`=20creates?= =?UTF-8?q?=20too=20many=20Threads=20when=20WebSo=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Thanks for submitting a PR! Please read these instructions carefully: - [x] Explain the **motivation** for making this change. - [ ] Provide a **test plan** demonstrating that the code is solid. - [ ] Match the **code formatting** of the rest of the codebase. - [ ] Target the `master` branch, NOT a "stable" branch. InspectorPackagerConnection now creates a new OkHttpClient when previous connection fails. If the failures occur frequently, many Threads are created in `WebSocketCall.enqueue()`. On my Pixel phone, I have seen up to 260 Threads named "OkHttp ConnectionPool" alive at the same time. So, why don't we consider reusing the existing OkHttpClient instance ? N/A Sign the [CLA][2], if you haven't already. Small pull requests are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it. Make sure all **tests pass** on both [Travis][3] and [Circle CI][4]. PRs that break tests are unlikely to be merged. For more info, see the ["Pull Requests"][5] section of our "Contributing" guidelines. [1]: https://medium.com/martinkonicek/what-is-a-test-plan-8bfc840ec171#.y9lcuqqi9 [2]: https://code.facebook.com/cla [3]: https://travis-ci.org/facebook/react-native [4]: http://circleci.com/gh/facebook/react-native [5]: https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#pull-requests Closes https://github.com/facebook/react-native/pull/14301 Differential Revision: D5172277 Pulled By: javache fbshipit-source-id: 7d417fa0675eb627f0b1ca41847b75686c8d1f3e --- .../devsupport/InspectorPackagerConnection.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/InspectorPackagerConnection.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/InspectorPackagerConnection.java index 04843ae05..3172191b2 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/InspectorPackagerConnection.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/InspectorPackagerConnection.java @@ -176,6 +176,7 @@ public class InspectorPackagerConnection { private final String mUrl; + private OkHttpClient mHttpClient; private @Nullable WebSocket mWebSocket; private final Handler mHandler; private boolean mClosed; @@ -223,14 +224,16 @@ public class InspectorPackagerConnection { if (mClosed) { throw new IllegalStateException("Can't connect closed client"); } - OkHttpClient httpClient = new OkHttpClient.Builder() - .connectTimeout(10, TimeUnit.SECONDS) - .writeTimeout(10, TimeUnit.SECONDS) - .readTimeout(0, TimeUnit.MINUTES) // Disable timeouts for read - .build(); + if (mHttpClient == null) { + mHttpClient = new OkHttpClient.Builder() + .connectTimeout(10, TimeUnit.SECONDS) + .writeTimeout(10, TimeUnit.SECONDS) + .readTimeout(0, TimeUnit.MINUTES) // Disable timeouts for read + .build(); + } Request request = new Request.Builder().url(mUrl).build(); - httpClient.newWebSocket(request, this); + mHttpClient.newWebSocket(request, this); } private void reconnect() {