Skip to content

Commit

Permalink
Fix InspectorPackagerConnection creates too many Threads when WebSo…
Browse files Browse the repository at this point in the history
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 #14301

Differential Revision: D5172277

Pulled By: javache

fbshipit-source-id: 7d417fa0675eb627f0b1ca41847b75686c8d1f3e
  • Loading branch information
xiaenlong authored and facebook-github-bot committed Jun 2, 2017
1 parent 79500f8 commit 70b3f2a
Showing 1 changed file with 9 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ private class Connection extends WebSocketListener {

private final String mUrl;

private OkHttpClient mHttpClient;
private @Nullable WebSocket mWebSocket;
private final Handler mHandler;
private boolean mClosed;
Expand Down Expand Up @@ -223,14 +224,16 @@ public void connect() {
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() {
Expand Down

0 comments on commit 70b3f2a

Please sign in to comment.