Force the debugger to disconnect before a bundle reload

Reviewed By: bnham Differential Revision: D5594238 fbshipit-source-id: feff9f179534c8e617f8fa7c8a7b1bc525c07cae
facebook · Aug 14, 2017 · 4150410 · 4150410
1 parent f11f001
commit 4150410
Show file tree

Hide file tree

Showing 12 changed files with 67 additions and 2 deletions.
diff --git a/Libraries/WebSocket/RCTReconnectingWebSocket.h b/Libraries/WebSocket/RCTReconnectingWebSocket.h
@@ -25,6 +25,8 @@
 
 - (instancetype)initWithURL:(NSURL *)url;
 @property (nonatomic, weak) id<RCTWebSocketProtocolDelegate> 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
@@ -70,7 +70,9 @@ - (void)start
   [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
@@ -20,6 +20,7 @@
 @interface RCTWebSocketObserver : NSObject
 
 - (instancetype)initWithURL:(NSURL *)url;
+- (void)setDelegateDispatchQueue:(dispatch_queue_t)queue;
 
 @property (nonatomic, weak) id<RCTWebSocketObserverDelegate> delegate;
 

diff --git a/Libraries/WebSocket/RCTWebSocketObserver.m b/Libraries/WebSocket/RCTWebSocketObserver.m
@@ -36,6 +36,11 @@ - (instancetype)initWithURL:(NSURL *)url
   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
@@ -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 @@ - (void)whitelistedModulesDidChange
 
 - (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
@@ -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
@@ -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 @@ @implementation RCTInspectorDevServerHelper
 
 RCT_NOT_IMPLEMENTED(- (instancetype)init)
 
+static NSMutableDictionary<NSString *, RCTInspectorPackagerConnection *> *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 @@ + (void)connectForContext:(JSGlobalContextRef)context
   // 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<NSString *, RCTInspectorPackagerConnection *> *socketConnections = nil;
   if (socketConnections == nil) {
     socketConnections = [NSMutableDictionary new];
   }

diff --git 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
@@ -74,6 +74,13 @@ - (void)handleProxyMessage:(NSDictionary<NSString *, id> *)message
   }
 }
 
+- (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
@@ -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 void openInspector(String id) {
     }
   }
 
+  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<Void, Void, Void>() {
       @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
@@ -679,6 +679,8 @@ public void isPackagerRunning(PackagerStatusCallback callback) {
 
   @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
@@ -8,6 +8,7 @@
 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 void closeQuietly() {
     mConnection.close();
   }
 
+  public void sendEventToAllConnections(String event) {
+    for (Map.Entry<String, Inspector.LocalConnection> inspectorConnectionEntry :
+        mInspectorConnections.entrySet()) {
+      Inspector.LocalConnection inspectorConnection = inspectorConnectionEntry.getValue();
+      inspectorConnection.sendMessage(event);
+    }
+  }
+
   public void sendOpenEvent(String pageId) {
     try {
       JSONObject payload = makePageIdPayload(pageId);