Skip to content

Commit

Permalink
networking: Fix requests failing when turning network off and on face…
Browse files Browse the repository at this point in the history
…book#19709.

This bug is probably actually a bug in OkHttp: square/okhttp#4079
Both issues linked above contain extensive details about the issue, its likely origins and
how to reproduce it. A short summary of the issue and the fix in this commit:

On Android, disconnecting from the network somehow corrupts the idle connections and ongoing
calls in okhttp clients. New requests made over these clients fail. This commit works around
that bug by evicting the idle connection pool and cancelling all ongoing calls of each client
when we receive a DISCONNECTED or CONNECTING event (we don't know yet if only one or both of
them cause the issue).
Cancelling all calls is aggressive, but when a device disconnects any ongoing calls can fail
anyway, so an app has to expect this scenario.
  • Loading branch information
roberthoenig committed Jun 27, 2018
1 parent 370bcff commit 0f8c285
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,18 @@

import javax.annotation.Nullable;

import android.Manifest;
import android.app.Activity;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.os.Bundle;
import android.support.v4.content.ContextCompat;
import android.view.KeyEvent;

import com.facebook.react.modules.core.DefaultHardwareBackBtnHandler;
import com.facebook.react.modules.core.PermissionAwareActivity;
import com.facebook.react.modules.core.PermissionListener;
import com.facebook.react.modules.network.OkHttpClientProvider;

/**
* Base Activity for React Native applications.
Expand Down Expand Up @@ -50,6 +54,10 @@ protected ReactActivityDelegate createReactActivityDelegate() {
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
mDelegate.onCreate(savedInstanceState);
if (ContextCompat.checkSelfPermission(this, Manifest.permission.ACCESS_NETWORK_STATE)
== PackageManager.PERMISSION_GRANTED) {
OkHttpClientProvider.addNetworkListenerToEvictIdleConnectionsOnNetworkChange(getApplicationContext());
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,25 @@

package com.facebook.react.modules.network;

import android.content.BroadcastReceiver;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.net.ConnectivityManager;
import android.net.NetworkInfo;
import android.os.AsyncTask;
import android.os.Build;
import android.os.Bundle;

import com.facebook.common.logging.FLog;
import com.facebook.react.ReactActivity;
import com.facebook.react.bridge.ReactApplicationContext;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.concurrent.TimeUnit;

import javax.annotation.Nullable;
Expand All @@ -33,6 +46,9 @@ public class OkHttpClientProvider {
// User-provided OkHttpClient factory
private static @Nullable OkHttpClientFactory sFactory;

private final static Set<OkHttpClient> sClients = Collections.newSetFromMap(
new WeakHashMap<OkHttpClient, Boolean>());

public static void setOkHttpClientFactory(OkHttpClientFactory factory) {
sFactory = factory;
}
Expand All @@ -43,6 +59,39 @@ public static OkHttpClient getOkHttpClient() {
}
return sClient;
}

private static class EvictIdleConnectionsTask extends AsyncTask {
@Override
protected Object doInBackground(Object[] objects) {
for (OkHttpClient client: sClients) {
client.connectionPool().evictAll();
client.dispatcher().cancelAll();
}
return null;
}
}

/*
See https://github.com/facebook/react-native/issues/19709 for context.
We know that idle connections get corrupted when the connectivity state
changes to disconnected, but the debugging of this issue hasn't been
exhaustive and it's possible that other changes in connectivity also
corrupt idle connections. `CONNECTIVITY_ACTION`s occur infrequently
enough to go safe and evict all idle connections on every such action.
*/
public static void addNetworkListenerToEvictIdleConnectionsOnNetworkChange(Context context) {
final BroadcastReceiver br = new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
if (intent.getAction().equals(ConnectivityManager.CONNECTIVITY_ACTION)) {
new EvictIdleConnectionsTask().execute();
}
}
};
final IntentFilter intentFilter = new IntentFilter();
intentFilter.addAction(ConnectivityManager.CONNECTIVITY_ACTION);
context.registerReceiver(br, intentFilter);
}

// okhttp3 OkHttpClient is immutable
// This allows app to init an OkHttpClient with custom settings.
Expand All @@ -54,7 +103,11 @@ public static OkHttpClient createClient() {
if (sFactory != null) {
return sFactory.createNewNetworkModuleClient();
}
return createClientBuilder().build();
else {
final OkHttpClient client = createClientBuilder().build();
registerClientToEvictIdleConnectionsOnNetworkChange(client);
return client;
}
}

public static OkHttpClient.Builder createClientBuilder() {
Expand All @@ -68,6 +121,10 @@ public static OkHttpClient.Builder createClientBuilder() {
return enableTls12OnPreLollipop(client);
}

public static void registerClientToEvictIdleConnectionsOnNetworkChange(OkHttpClient client) {
sClients.add(client);
}

/*
On Android 4.1-4.4 (API level 16 to 19) TLS 1.1 and 1.2 are
available but not enabled by default. The following method
Expand Down

0 comments on commit 0f8c285

Please sign in to comment.