Skip to content

Commit 16ad818

Browse files
motiz88facebook-github-bot
authored andcommitted
Handle all incoming Inspector messages on main thread, downgrade some errors to logs
Summary: Changelog: [Internal] * Updates `InspectorPackagerConnection.java`, `DevServerHelper.java` and `DevSupportManagerBase.java` to perform all connection management and message dispatching for the inspector socket on the main thread. This is in support of a new CDP implementation in React Native that will strictly assume it's called on the main thread (thus avoiding the need for explicit locking in many places). * Downgrades JSON parsing errors and duplicate connection errors from exceptions to logs, matching the [iOS implementation](https://github.com/facebook/react-native/blob/main/packages/react-native/React/Inspector/RCTInspectorPackagerConnection.m). Reviewed By: javache Differential Revision: D51346658 fbshipit-source-id: 3d0d5588a824c1b28da5499ef9d040998a941288
1 parent 7625a50 commit 16ad818

File tree

3 files changed

+64
-54
lines changed

3 files changed

+64
-54
lines changed

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -210,16 +210,10 @@ public void openInspectorConnection() {
210210
FLog.w(ReactConstants.TAG, "Inspector connection already open, nooping.");
211211
return;
212212
}
213-
new AsyncTask<Void, Void, Void>() {
214-
@Override
215-
protected Void doInBackground(Void... params) {
216-
mInspectorPackagerConnection =
217-
new InspectorPackagerConnection(
218-
getInspectorDeviceUrl(), mPackageName, mBundlerStatusProvider);
219-
mInspectorPackagerConnection.connect();
220-
return null;
221-
}
222-
}.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
213+
mInspectorPackagerConnection =
214+
new InspectorPackagerConnection(
215+
getInspectorDeviceUrl(), mPackageName, mBundlerStatusProvider);
216+
mInspectorPackagerConnection.connect();
223217
}
224218

225219
public void disableDebugger() {
@@ -229,16 +223,10 @@ public void disableDebugger() {
229223
}
230224

231225
public void closeInspectorConnection() {
232-
new AsyncTask<Void, Void, Void>() {
233-
@Override
234-
protected Void doInBackground(Void... params) {
235-
if (mInspectorPackagerConnection != null) {
236-
mInspectorPackagerConnection.closeQuietly();
237-
mInspectorPackagerConnection = null;
238-
}
239-
return null;
240-
}
241-
}.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
226+
if (mInspectorPackagerConnection != null) {
227+
mInspectorPackagerConnection.closeQuietly();
228+
mInspectorPackagerConnection = null;
229+
}
242230
}
243231

244232
public String getWebsocketProxyURL() {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -942,13 +942,15 @@ private void reportBundleLoadingFailure(final Exception cause) {
942942

943943
@Override
944944
public void startInspector() {
945+
UiThreadUtil.assertOnUiThread();
945946
if (mIsDevSupportEnabled) {
946947
mDevServerHelper.openInspectorConnection();
947948
}
948949
}
949950

950951
@Override
951952
public void stopInspector() {
953+
UiThreadUtil.assertOnUiThread();
952954
mDevServerHelper.closeInspectorConnection();
953955
}
954956

@@ -1045,9 +1047,12 @@ public void onPackagerDisconnected() {
10451047

10461048
@Override
10471049
public void onPackagerReloadCommand() {
1048-
// Disable debugger to resume the JsVM & avoid thread locks while reloading
1049-
mDevServerHelper.disableDebugger();
1050-
UiThreadUtil.runOnUiThread(() -> handleReloadJS());
1050+
UiThreadUtil.runOnUiThread(
1051+
() -> {
1052+
// Disable debugger to resume the JsVM & avoid thread locks while reloading
1053+
mDevServerHelper.disableDebugger();
1054+
handleReloadJS();
1055+
});
10511056
}
10521057

10531058
@Override

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/InspectorPackagerConnection.java

Lines changed: 48 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,10 @@
88
package com.facebook.react.devsupport;
99

1010
import android.os.AsyncTask;
11-
import android.os.Handler;
12-
import android.os.Looper;
1311
import androidx.annotation.Nullable;
1412
import com.facebook.common.logging.FLog;
1513
import com.facebook.react.bridge.Inspector;
16-
import java.io.IOException;
14+
import com.facebook.react.bridge.UiThreadUtil;
1715
import java.util.HashMap;
1816
import java.util.List;
1917
import java.util.Map;
@@ -37,17 +35,20 @@ public class InspectorPackagerConnection {
3735

3836
public InspectorPackagerConnection(
3937
String url, String packageName, BundleStatusProvider bundleStatusProvider) {
38+
UiThreadUtil.assertOnUiThread();
4039
mConnection = new Connection(url);
4140
mInspectorConnections = new HashMap<>();
4241
mPackageName = packageName;
4342
mBundleStatusProvider = bundleStatusProvider;
4443
}
4544

4645
public void connect() {
46+
UiThreadUtil.assertOnUiThread();
4747
mConnection.connect();
4848
}
4949

5050
public void closeQuietly() {
51+
UiThreadUtil.assertOnUiThread();
5152
mConnection.close();
5253
}
5354

@@ -59,7 +60,7 @@ public void sendEventToAllConnections(String event) {
5960
}
6061
}
6162

62-
void handleProxyMessage(JSONObject message) throws JSONException, IOException {
63+
void handleProxyMessage(JSONObject message) throws JSONException {
6364
String event = message.getString("event");
6465
switch (event) {
6566
case "getPages":
@@ -75,7 +76,8 @@ void handleProxyMessage(JSONObject message) throws JSONException, IOException {
7576
handleDisconnect(message.getJSONObject("payload"));
7677
break;
7778
default:
78-
throw new IllegalArgumentException("Unknown event: " + event);
79+
FLog.e(TAG, "Unknown event: " + event);
80+
break;
7981
}
8082
}
8183

@@ -90,7 +92,8 @@ private void handleConnect(JSONObject payload) throws JSONException {
9092
final String pageId = payload.getString("pageId");
9193
Inspector.LocalConnection inspectorConnection = mInspectorConnections.remove(pageId);
9294
if (inspectorConnection != null) {
93-
throw new IllegalStateException("Already connected: " + pageId);
95+
FLog.w(TAG, "Already connected: " + pageId);
96+
return;
9497
}
9598

9699
try {
@@ -191,49 +194,65 @@ private class Connection extends WebSocketListener {
191194

192195
private OkHttpClient mHttpClient;
193196
private @Nullable WebSocket mWebSocket;
194-
private final Handler mHandler;
195197
private boolean mClosed;
196198
private boolean mSuppressConnectionErrors;
197199

198200
public Connection(String url) {
199201
mUrl = url;
200-
mHandler = new Handler(Looper.getMainLooper());
201202
}
202203

203204
@Override
204205
public void onOpen(WebSocket webSocket, Response response) {
205-
mWebSocket = webSocket;
206+
UiThreadUtil.runOnUiThread(
207+
() -> {
208+
mWebSocket = webSocket;
209+
});
206210
}
207211

208212
@Override
209213
public void onFailure(WebSocket webSocket, Throwable t, Response response) {
210-
if (mWebSocket != null) {
211-
abort("Websocket exception", t);
212-
}
213-
if (!mClosed) {
214-
reconnect();
215-
}
214+
UiThreadUtil.runOnUiThread(
215+
() -> {
216+
if (mWebSocket != null) {
217+
abort("Websocket exception", t);
218+
}
219+
if (!mClosed) {
220+
reconnect();
221+
}
222+
});
216223
}
217224

218225
@Override
219226
public void onMessage(WebSocket webSocket, String text) {
220227
try {
221-
handleProxyMessage(new JSONObject(text));
222-
} catch (Exception e) {
223-
throw new RuntimeException(e);
228+
JSONObject parsed = new JSONObject(text);
229+
UiThreadUtil.runOnUiThread(
230+
() -> {
231+
try {
232+
handleProxyMessage(parsed);
233+
} catch (JSONException e) {
234+
FLog.w(TAG, "Error handling inspector message", e);
235+
}
236+
});
237+
} catch (JSONException e) {
238+
FLog.w(TAG, "Unrecognized inspector message, string was not valid JSON: " + text, e);
224239
}
225240
}
226241

227242
@Override
228243
public void onClosed(WebSocket webSocket, int code, String reason) {
229-
mWebSocket = null;
230-
closeAllConnections();
231-
if (!mClosed) {
232-
reconnect();
233-
}
244+
UiThreadUtil.runOnUiThread(
245+
() -> {
246+
mWebSocket = null;
247+
closeAllConnections();
248+
if (!mClosed) {
249+
reconnect();
250+
}
251+
});
234252
}
235253

236254
public void connect() {
255+
UiThreadUtil.assertOnUiThread();
237256
if (mClosed) {
238257
throw new IllegalStateException("Can't connect closed client");
239258
}
@@ -258,20 +277,18 @@ private void reconnect() {
258277
FLog.w(TAG, "Couldn't connect to packager, will silently retry");
259278
mSuppressConnectionErrors = true;
260279
}
261-
mHandler.postDelayed(
262-
new Runnable() {
263-
@Override
264-
public void run() {
265-
// check that we haven't been closed in the meantime
266-
if (!mClosed) {
267-
connect();
268-
}
280+
UiThreadUtil.runOnUiThread(
281+
() -> {
282+
// check that we haven't been closed in the meantime
283+
if (!mClosed) {
284+
connect();
269285
}
270286
},
271287
RECONNECT_DELAY_MS);
272288
}
273289

274290
public void close() {
291+
UiThreadUtil.assertOnUiThread();
275292
mClosed = true;
276293
if (mWebSocket != null) {
277294
try {

0 commit comments

Comments
 (0)