Skip to content

Commit dbdae56

Browse files
sjuddglide-copybara-robot
sjudd
authored and
glide-copybara-robot
committed
Make the connectivity BroadcastReceiver in Glide a singleton
Fixes #1161 PiperOrigin-RevId: 397401635
1 parent e9f7cec commit dbdae56

File tree

3 files changed

+170
-80
lines changed

3 files changed

+170
-80
lines changed

‎library/src/main/java/com/bumptech/glide/manager/DefaultConnectivityMonitor.java

+7-79
Original file line numberDiff line numberDiff line change
@@ -1,104 +1,32 @@
11
package com.bumptech.glide.manager;
22

3-
import android.annotation.SuppressLint;
4-
import android.content.BroadcastReceiver;
53
import android.content.Context;
6-
import android.content.Intent;
7-
import android.content.IntentFilter;
8-
import android.net.ConnectivityManager;
9-
import android.net.NetworkInfo;
10-
import android.util.Log;
114
import androidx.annotation.NonNull;
12-
import com.bumptech.glide.util.Preconditions;
135
import com.bumptech.glide.util.Synthetic;
146

15-
/** Uses {@link android.net.ConnectivityManager} to identify connectivity changes. */
7+
/**
8+
* An Android Lifecycle wrapper that uses {@link SingletonConnectivityReceiver} to observer
9+
* connectivity changes, allowing for registration to be removed when our listener is being
10+
* destroyed as part of the Android lifecycle.
11+
*/
1612
final class DefaultConnectivityMonitor implements ConnectivityMonitor {
17-
private static final String TAG = "ConnectivityMonitor";
1813
private final Context context;
1914

2015
@SuppressWarnings("WeakerAccess")
2116
@Synthetic
2217
final ConnectivityListener listener;
2318

24-
@SuppressWarnings("WeakerAccess")
25-
@Synthetic
26-
boolean isConnected;
27-
28-
private boolean isRegistered;
29-
30-
private final BroadcastReceiver connectivityReceiver =
31-
new BroadcastReceiver() {
32-
@Override
33-
public void onReceive(@NonNull Context context, Intent intent) {
34-
boolean wasConnected = isConnected;
35-
isConnected = isConnected(context);
36-
if (wasConnected != isConnected) {
37-
if (Log.isLoggable(TAG, Log.DEBUG)) {
38-
Log.d(TAG, "connectivity changed, isConnected: " + isConnected);
39-
}
40-
41-
listener.onConnectivityChanged(isConnected);
42-
}
43-
}
44-
};
45-
4619
DefaultConnectivityMonitor(@NonNull Context context, @NonNull ConnectivityListener listener) {
4720
this.context = context.getApplicationContext();
4821
this.listener = listener;
4922
}
5023

5124
private void register() {
52-
if (isRegistered) {
53-
return;
54-
}
55-
56-
// Initialize isConnected.
57-
isConnected = isConnected(context);
58-
try {
59-
// See #1405
60-
context.registerReceiver(
61-
connectivityReceiver, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
62-
isRegistered = true;
63-
} catch (SecurityException e) {
64-
// See #1417, registering the receiver can throw SecurityException.
65-
if (Log.isLoggable(TAG, Log.WARN)) {
66-
Log.w(TAG, "Failed to register", e);
67-
}
68-
}
25+
SingletonConnectivityReceiver.get(context).register(listener);
6926
}
7027

7128
private void unregister() {
72-
if (!isRegistered) {
73-
return;
74-
}
75-
76-
context.unregisterReceiver(connectivityReceiver);
77-
isRegistered = false;
78-
}
79-
80-
@SuppressWarnings("WeakerAccess")
81-
@Synthetic
82-
// Permissions are checked in the factory instead.
83-
@SuppressLint("MissingPermission")
84-
boolean isConnected(@NonNull Context context) {
85-
ConnectivityManager connectivityManager =
86-
Preconditions.checkNotNull(
87-
(ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE));
88-
NetworkInfo networkInfo;
89-
try {
90-
networkInfo = connectivityManager.getActiveNetworkInfo();
91-
} catch (RuntimeException e) {
92-
// #1405 shows that this throws a SecurityException.
93-
// b/70869360 shows that this throws NullPointerException on APIs 22, 23, and 24.
94-
// b/70869360 also shows that this throws RuntimeException on API 24 and 25.
95-
if (Log.isLoggable(TAG, Log.WARN)) {
96-
Log.w(TAG, "Failed to determine connectivity status when connectivity changed", e);
97-
}
98-
// Default to true;
99-
return true;
100-
}
101-
return networkInfo != null && networkInfo.isConnected();
29+
SingletonConnectivityReceiver.get(context).unregister(listener);
10230
}
10331

10432
@Override

‎library/src/main/java/com/bumptech/glide/manager/SingletonConnectivityReceiver.java

+156
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
package com.bumptech.glide.manager;
2+
3+
import android.annotation.SuppressLint;
4+
import android.content.BroadcastReceiver;
5+
import android.content.Context;
6+
import android.content.Intent;
7+
import android.content.IntentFilter;
8+
import android.net.ConnectivityManager;
9+
import android.net.NetworkInfo;
10+
import android.util.Log;
11+
import androidx.annotation.GuardedBy;
12+
import androidx.annotation.NonNull;
13+
import androidx.annotation.VisibleForTesting;
14+
import com.bumptech.glide.manager.ConnectivityMonitor.ConnectivityListener;
15+
import com.bumptech.glide.util.Preconditions;
16+
import com.bumptech.glide.util.Synthetic;
17+
import java.util.ArrayList;
18+
import java.util.HashSet;
19+
import java.util.List;
20+
import java.util.Set;
21+
22+
/** Uses {@link android.net.ConnectivityManager} to identify connectivity changes. */
23+
final class SingletonConnectivityReceiver {
24+
private static volatile SingletonConnectivityReceiver instance;
25+
@Synthetic static final String TAG = "ConnectivityMonitor";
26+
// Only accessed on the main thread.
27+
@Synthetic boolean isConnected;
28+
29+
private final BroadcastReceiver connectivityReceiver =
30+
new BroadcastReceiver() {
31+
@Override
32+
public void onReceive(@NonNull Context context, Intent intent) {
33+
List<ConnectivityListener> listenersToNotify = null;
34+
boolean wasConnected = isConnected;
35+
isConnected = isConnected(context);
36+
if (wasConnected != isConnected) {
37+
if (Log.isLoggable(TAG, Log.DEBUG)) {
38+
Log.d(TAG, "connectivity changed, isConnected: " + isConnected);
39+
}
40+
41+
synchronized (SingletonConnectivityReceiver.this) {
42+
listenersToNotify = new ArrayList<>(listeners);
43+
}
44+
}
45+
// Make sure that we do not hold our lock while calling our listener. Otherwise we could
46+
// deadlock where our listener acquires its lock, then tries to acquire ours elsewhere and
47+
// then here we acquire our lock and try to acquire theirs.
48+
// The consequence of this is that we may notify a listener after it has been
49+
// unregistered in a few specific (unlikely) scenarios. That appears to be safe and is
50+
// documented in the unregister method.
51+
if (listenersToNotify != null) {
52+
for (ConnectivityListener listener : listenersToNotify) {
53+
listener.onConnectivityChanged(isConnected);
54+
}
55+
}
56+
}
57+
};
58+
59+
private final Context context;
60+
61+
@GuardedBy("this")
62+
@Synthetic
63+
final Set<ConnectivityListener> listeners = new HashSet<ConnectivityListener>();
64+
65+
@GuardedBy("this")
66+
private boolean isRegistered;
67+
68+
static SingletonConnectivityReceiver get(@NonNull Context context) {
69+
if (instance == null) {
70+
synchronized (SingletonConnectivityReceiver.class) {
71+
if (instance == null) {
72+
instance = new SingletonConnectivityReceiver(context);
73+
}
74+
}
75+
}
76+
return instance;
77+
}
78+
79+
@VisibleForTesting
80+
static void reset() {
81+
instance = null;
82+
}
83+
84+
private SingletonConnectivityReceiver(@NonNull Context context) {
85+
this.context = context.getApplicationContext();
86+
}
87+
88+
synchronized void register(ConnectivityListener listener) {
89+
listeners.add(listener);
90+
maybeRegisterReceiver();
91+
}
92+
93+
/**
94+
* To avoid holding a lock while notifying listeners, the unregisterd listener may still be
95+
* notified about a connectivity change after this method completes if this method is called on a
96+
* thread other than the main thread and if a connectivity broadcast is racing with this method.
97+
* Callers must handle this case.
98+
*/
99+
synchronized void unregister(ConnectivityListener listener) {
100+
listeners.remove(listener);
101+
maybeUnregisterReceiver();
102+
}
103+
104+
@GuardedBy("this")
105+
private void maybeRegisterReceiver() {
106+
if (isRegistered || listeners.isEmpty()) {
107+
return;
108+
}
109+
isConnected = isConnected(context);
110+
try {
111+
// See #1405
112+
context.registerReceiver(
113+
connectivityReceiver, new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
114+
isRegistered = true;
115+
} catch (SecurityException e) {
116+
// See #1417, registering the receiver can throw SecurityException.
117+
if (Log.isLoggable(TAG, Log.WARN)) {
118+
Log.w(TAG, "Failed to register", e);
119+
}
120+
}
121+
}
122+
123+
@GuardedBy("this")
124+
private void maybeUnregisterReceiver() {
125+
if (!isRegistered || !listeners.isEmpty()) {
126+
return;
127+
}
128+
129+
context.unregisterReceiver(connectivityReceiver);
130+
isRegistered = false;
131+
}
132+
133+
@SuppressWarnings("WeakerAccess")
134+
@Synthetic
135+
// Permissions are checked in the factory instead.
136+
@SuppressLint("MissingPermission")
137+
boolean isConnected(@NonNull Context context) {
138+
ConnectivityManager connectivityManager =
139+
Preconditions.checkNotNull(
140+
(ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE));
141+
NetworkInfo networkInfo;
142+
try {
143+
networkInfo = connectivityManager.getActiveNetworkInfo();
144+
} catch (RuntimeException e) {
145+
// #1405 shows that this throws a SecurityException.
146+
// b/70869360 shows that this throws NullPointerException on APIs 22, 23, and 24.
147+
// b/70869360 also shows that this throws RuntimeException on API 24 and 25.
148+
if (Log.isLoggable(TAG, Log.WARN)) {
149+
Log.w(TAG, "Failed to determine connectivity status when connectivity changed", e);
150+
}
151+
// Default to true;
152+
return true;
153+
}
154+
return networkInfo != null && networkInfo.isConnected();
155+
}
156+
}

‎library/test/src/test/java/com/bumptech/glide/manager/DefaultConnectivityMonitorTest.java

+7-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import androidx.test.core.app.ApplicationProvider;
1818
import com.bumptech.glide.manager.DefaultConnectivityMonitorTest.PermissionConnectivityManager;
1919
import java.util.List;
20+
import org.junit.After;
2021
import org.junit.Before;
2122
import org.junit.Test;
2223
import org.junit.runner.RunWith;
@@ -46,6 +47,11 @@ public void setUp() {
4647
harness = new ConnectivityHarness();
4748
}
4849

50+
@After
51+
public void tearDown() {
52+
SingletonConnectivityReceiver.reset();
53+
}
54+
4955
@Test
5056
public void testRegistersReceiverOnStart() {
5157
monitor.onStart();
@@ -106,7 +112,7 @@ public void testNotifiesListenerIfDisconnectedAndBecomesConnected() {
106112
harness.connect();
107113
harness.broadcast();
108114

109-
verify(listener).onConnectivityChanged(eq(true));
115+
verify(listener).onConnectivityChanged(true);
110116
}
111117

112118
@Test

0 commit comments

Comments
 (0)