From 41504103ceecbaa2d6659c42bb99a07960472fcb Mon Sep 17 00:00:00 2001 From: Paco Estevez Garcia Date: Mon, 14 Aug 2017 08:07:13 -0700 Subject: [PATCH] Force the debugger to disconnect before a bundle reload Reviewed By: bnham Differential Revision: D5594238 fbshipit-source-id: feff9f179534c8e617f8fa7c8a7b1bc525c07cae --- Libraries/WebSocket/RCTReconnectingWebSocket.h | 2 ++ Libraries/WebSocket/RCTReconnectingWebSocket.m | 4 +++- Libraries/WebSocket/RCTWebSocketObserver.h | 1 + Libraries/WebSocket/RCTWebSocketObserver.m | 5 +++++ React/Base/RCTBridge.m | 6 ++++++ React/DevSupport/RCTInspectorDevServerHelper.h | 1 + React/DevSupport/RCTInspectorDevServerHelper.mm | 17 ++++++++++++++++- .../Inspector/RCTInspectorPackagerConnection.h | 1 + .../Inspector/RCTInspectorPackagerConnection.m | 7 +++++++ .../react/devsupport/DevServerHelper.java | 14 ++++++++++++++ .../react/devsupport/DevSupportManagerImpl.java | 2 ++ .../devsupport/InspectorPackagerConnection.java | 9 +++++++++ 12 files changed, 67 insertions(+), 2 deletions(-) diff --git a/Libraries/WebSocket/RCTReconnectingWebSocket.h b/Libraries/WebSocket/RCTReconnectingWebSocket.h index af61f81d1..7466d73d9 100644 --- a/Libraries/WebSocket/RCTReconnectingWebSocket.h +++ b/Libraries/WebSocket/RCTReconnectingWebSocket.h @@ -25,6 +25,8 @@ - (instancetype)initWithURL:(NSURL *)url; @property (nonatomic, weak) id delegate; +/** @brief Must be set before -start to have effect */ +@property (nonatomic, strong) dispatch_queue_t delegateDispatchQueue; - (void)send:(id)data; - (void)start; - (void)stop; diff --git a/Libraries/WebSocket/RCTReconnectingWebSocket.m b/Libraries/WebSocket/RCTReconnectingWebSocket.m index 0c4a5c163..f928c9804 100644 --- a/Libraries/WebSocket/RCTReconnectingWebSocket.m +++ b/Libraries/WebSocket/RCTReconnectingWebSocket.m @@ -70,7 +70,9 @@ static void my_nwlog_legacy_v(int level, char *format, va_list args) { [self stop]; _socket = [[RCTSRWebSocket alloc] initWithURL:_url]; _socket.delegate = self; - + if (_delegateDispatchQueue) { + [_socket setDelegateDispatchQueue:_delegateDispatchQueue]; + } [_socket open]; } diff --git a/Libraries/WebSocket/RCTWebSocketObserver.h b/Libraries/WebSocket/RCTWebSocketObserver.h index 4969876b4..e20c232d9 100644 --- a/Libraries/WebSocket/RCTWebSocketObserver.h +++ b/Libraries/WebSocket/RCTWebSocketObserver.h @@ -20,6 +20,7 @@ @interface RCTWebSocketObserver : NSObject - (instancetype)initWithURL:(NSURL *)url; +- (void)setDelegateDispatchQueue:(dispatch_queue_t)queue; @property (nonatomic, weak) id delegate; diff --git a/Libraries/WebSocket/RCTWebSocketObserver.m b/Libraries/WebSocket/RCTWebSocketObserver.m index fc4d9d400..af2f16c05 100644 --- a/Libraries/WebSocket/RCTWebSocketObserver.m +++ b/Libraries/WebSocket/RCTWebSocketObserver.m @@ -36,6 +36,11 @@ return self; } +- (void)setDelegateDispatchQueue:(dispatch_queue_t)queue +{ + [_socket setDelegateDispatchQueue:queue]; +} + - (void)start { _socket.delegate = self; diff --git a/React/Base/RCTBridge.m b/React/Base/RCTBridge.m index 6a980d970..8173d8d1c 100644 --- a/React/Base/RCTBridge.m +++ b/React/Base/RCTBridge.m @@ -14,6 +14,7 @@ #import "RCTConvert.h" #import "RCTEventDispatcher.h" +#import "RCTInspectorDevServerHelper.h" #import "RCTJSEnvironment.h" #import "RCTLog.h" #import "RCTModuleData.h" @@ -252,6 +253,11 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init) - (void)reload { + #if ENABLE_INSPECTOR + // Disable debugger to resume the JsVM & avoid thread locks while reloading + [RCTInspectorDevServerHelper disableDebugger]; + #endif + /** * Any thread */ diff --git a/React/DevSupport/RCTInspectorDevServerHelper.h b/React/DevSupport/RCTInspectorDevServerHelper.h index ad7df214e..b107dbdc6 100644 --- a/React/DevSupport/RCTInspectorDevServerHelper.h +++ b/React/DevSupport/RCTInspectorDevServerHelper.h @@ -10,6 +10,7 @@ + (void)connectForContext:(JSGlobalContextRef)context withBundleURL:(NSURL *)bundleURL; ++ (void)disableDebugger; @end #endif diff --git a/React/DevSupport/RCTInspectorDevServerHelper.mm b/React/DevSupport/RCTInspectorDevServerHelper.mm index 8cda1b6ae..4f1357fa3 100644 --- a/React/DevSupport/RCTInspectorDevServerHelper.mm +++ b/React/DevSupport/RCTInspectorDevServerHelper.mm @@ -10,6 +10,8 @@ using namespace facebook::react; +static NSString *const kDebuggerMsgDisable = @"{ \"id\":1,\"method\":\"Debugger.disable\" }"; + static NSString *getDebugServerHost(NSURL *bundleURL) { NSString *host = [bundleURL host]; @@ -43,6 +45,20 @@ static NSURL *getInspectorDeviceUrl(NSURL *bundleURL) RCT_NOT_IMPLEMENTED(- (instancetype)init) +static NSMutableDictionary *socketConnections = nil; + +static void sendEventToAllConnections(NSString *event) +{ + for (NSString *socketId in socketConnections) { + [socketConnections[socketId] sendEventToAllConnections:event]; + } +} + ++ (void)disableDebugger +{ + sendEventToAllConnections(kDebuggerMsgDisable); +} + + (void)connectForContext:(JSGlobalContextRef)context withBundleURL:(NSURL *)bundleURL { @@ -55,7 +71,6 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init) // Note, using a static dictionary isn't really the greatest design, but // the packager connection does the same thing, so it's at least consistent. // This is a static map that holds different inspector clients per the inspectorURL - static NSMutableDictionary *socketConnections = nil; if (socketConnections == nil) { socketConnections = [NSMutableDictionary new]; } diff --git a/React/Inspector/RCTInspectorPackagerConnection.h b/React/Inspector/RCTInspectorPackagerConnection.h index 3ff8ebcfd..a36dff330 100644 --- a/React/Inspector/RCTInspectorPackagerConnection.h +++ b/React/Inspector/RCTInspectorPackagerConnection.h @@ -10,6 +10,7 @@ - (instancetype)initWithURL:(NSURL *)url; - (void)connect; - (void)closeQuietly; +- (void)sendEventToAllConnections:(NSString *)event; - (void)sendOpenEvent:(NSString *)pageId; @end diff --git a/React/Inspector/RCTInspectorPackagerConnection.m b/React/Inspector/RCTInspectorPackagerConnection.m index 06ed7daac..753c79f12 100644 --- a/React/Inspector/RCTInspectorPackagerConnection.m +++ b/React/Inspector/RCTInspectorPackagerConnection.m @@ -74,6 +74,13 @@ RCT_NOT_IMPLEMENTED(- (instancetype)init) } } +- (void)sendEventToAllConnections:(NSString *)event +{ + for (NSString *pageId in _inspectorConnections) { + [_inspectorConnections[pageId] sendMessage:event]; + } +} + - (void)closeAllConnections { for (NSString *pageId in _inspectorConnections){ diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java index 0faf18858..4af763a74 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java @@ -90,6 +90,8 @@ public class DevServerHelper { private static final int LONG_POLL_FAILURE_DELAY_MS = 5000; private static final int HTTP_CONNECT_TIMEOUT_MS = 5000; + private static final String DEBUGGER_MSG_DISABLE = "{ \"id\":1,\"method\":\"Debugger.disable\" }"; + public interface OnServerContentChangeListener { void onServerContentChanged(); } @@ -211,6 +213,18 @@ public class DevServerHelper { } } + public void sendEventToAllConnections(String event) { + if (mInspectorPackagerConnection != null) { + mInspectorPackagerConnection.sendEventToAllConnections(event); + } + } + + public void disableDebugger() { + if (mInspectorPackagerConnection != null) { + mInspectorPackagerConnection.sendEventToAllConnections(DEBUGGER_MSG_DISABLE); + } + } + public void closeInspectorConnection() { new AsyncTask() { @Override diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java index fcf248c46..b62c7d2e8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java @@ -679,6 +679,8 @@ public class DevSupportManagerImpl implements @Override public void onPackagerReloadCommand() { + // Disable debugger to resume the JsVM & avoid thread locks while reloading + mDevServerHelper.disableDebugger(); UiThreadUtil.runOnUiThread(new Runnable() { @Override public void run() { 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 b61c83579..591526176 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/InspectorPackagerConnection.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/InspectorPackagerConnection.java @@ -8,6 +8,7 @@ import java.io.IOException; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.concurrent.TimeUnit; import android.os.AsyncTask; @@ -47,6 +48,14 @@ public class InspectorPackagerConnection { mConnection.close(); } + public void sendEventToAllConnections(String event) { + for (Map.Entry inspectorConnectionEntry : + mInspectorConnections.entrySet()) { + Inspector.LocalConnection inspectorConnection = inspectorConnectionEntry.getValue(); + inspectorConnection.sendMessage(event); + } + } + public void sendOpenEvent(String pageId) { try { JSONObject payload = makePageIdPayload(pageId);