fix(inspector): Fix app crashes on second debugger attach

src/NativeScript/Calling/FFICall.cpp

@@ -176,7 +176,12 @@ JSObject* FFICall::async(ExecState* execState, JSValue thisValue, const ArgList&
 
       ffi_call(callee->_cif.get(), FFI_FN(invocation->function), invocation->resultBuffer(), reinterpret_cast<void**>(invocation->_buffer + callee->_argsArrayOffset));
 
-      JSLockHolder lockHolder(fakeExecState);
+      // Native call is made outside of the VM lock by design.
+      // For more information see https://github.com/NativeScript/ios-runtime/issues/215 and it's corresponding PR.
+      // This creates a racing condition which might corrupt the internal state of the VM but
+      // a fix for it is outside of this PR's scope, so I'm leaving it like it has always been.
+
+      JSLockHolder lockHolder(vm);
       // we no longer have a valid caller on the stack, what with being async and all
       fakeExecState->setCallerFrame(CallFrame::noCaller());
 

src/NativeScript/Calling/FFICallbackInlines.h

@@ -21,11 +21,11 @@ template <class DerivedCallback>
 inline void FFICallback<DerivedCallback>::ffiClosureCallback(ffi_cif* cif, void* retValue, void** argValues, void* userData) {
     FFICallback* callback = static_cast<FFICallback*>(userData);
     JSC::ExecState* execState = callback->_globalExecState;
-
     JSC::VM& vm = execState->vm();
+    JSC::JSLockHolder lock(vm);
+
     auto scope = DECLARE_CATCH_SCOPE(vm);
 
-    JSC::JSLockHolder lock(execState->vm());
     static_cast<DerivedCallback*>(callback)->ffiClosureCallback(retValue, argValues, userData);
 
     reportErrorIfAny(execState, scope);
@@ -120,6 +120,6 @@ inline FFICallback<DerivedCallback>::~FFICallback() {
     delete[] this->_cif->arg_types;
     delete this->_cif;
 }
-}
+} // namespace NativeScript
 
 #endif /* defined(__NativeScript__FFICallbackInlines__) */

src/NativeScript/GlobalObject.mm

@@ -49,8 +49,8 @@
 #include <JavaScriptCore/StrongInlines.h>
 #include <JavaScriptCore/inspector/JSGlobalObjectConsoleClient.h>
 #include <JavaScriptCore/runtime/VMEntryScope.h>
-#include <string>
 #include <chrono>
+#include <string>
 
 namespace NativeScript {
 using namespace JSC;
@@ -105,7 +105,7 @@ static EncodedJSValue JSC_HOST_CALL collectGarbage(ExecState* execState) {
     JSSynchronousGarbageCollectForDebugging(execState->lexicalGlobalObject()->globalExec());
     return JSValue::encode(jsUndefined());
 }
-    
+
 static EncodedJSValue JSC_HOST_CALL time(ExecState* execState) {
     auto nano = std::chrono::time_point_cast<std::chrono::nanoseconds>(std::chrono::steady_clock::now());
     double duration = nano.time_since_epoch().count() / 1000000.0;
@@ -120,6 +120,7 @@ static void microtaskRunLoopSourcePerformWork(void* context) {
 
 static void runLoopBeforeWaitingPerformWork(CFRunLoopObserverRef observer, CFRunLoopActivity activity, void* info) {
     GlobalObject* self = static_cast<GlobalObject*>(info);
+    JSC::JSLockHolder lock(self->vm());
     VMEntryScope* currentEntryScope = self->vm().entryScope;
     if (self->vm().topCallFrame && currentEntryScope && !currentEntryScope->didPopListeners().isEmpty()) {
         FFIFunctionCall* function_call = jsDynamicCast<FFIFunctionCall*>(self->vm().topCallFrame->callee());

src/NativeScript/ObjC/Enumeration/TNSFastEnumerationAdapter.mm

@@ -29,10 +29,10 @@ NSUInteger TNSFastEnumerationAdapter(id self, NSFastEnumerationState* state, id
         RELEASE_ASSERT(wrapper);
 
         JSC::VM& vm = execState->vm();
-        auto scope = DECLARE_CATCH_SCOPE(vm);
-
         JSLockHolder lock(execState);
 
+        auto scope = DECLARE_CATCH_SCOPE(vm);
+
         JSValue iteratorFunction = wrapper->get(execState, execState->propertyNames().iteratorSymbol);
         reportErrorIfAny(execState, scope);
 
@@ -60,8 +60,8 @@ NSUInteger TNSFastEnumerationAdapter(id self, NSFastEnumerationState* state, id
 
     ExecState* execState = reinterpret_cast<ExecState*>(state->extra[0]);
     JSValue iterator(reinterpret_cast<JSCell*>(state->extra[1]));
-    JSLockHolder lock(execState);
     JSC::VM& vm = execState->vm();
+    JSLockHolder lock(vm);
     auto scope = DECLARE_CATCH_SCOPE(vm);
 
     if (state->state == State::Done) {
@@ -95,4 +95,4 @@ NSUInteger TNSFastEnumerationAdapter(id self, NSFastEnumerationState* state, id
 
     return count;
 }
-}
+}

src/NativeScript/ObjC/Inheritance/ObjCTypeScriptExtend.mm

@@ -100,9 +100,9 @@ EncodedJSValue ObjCTypeScriptExtendFunction(ExecState* execState) {
       if (self != [derivedClass self]) {
           return;
       }
+      JSLockHolder lock(globalObject->vm());
       auto catchScope = DECLARE_CATCH_SCOPE(globalObject->vm());
 
-      JSLockHolder lock(globalObject->vm());
       ExecState* globalExec = globalObject->globalExec();
 
       JSObject* instanceMethods = jsCast<JSObject*>(derivedConstructor->get(globalExec, globalExec->vm().propertyNames->prototype));
@@ -121,4 +121,4 @@ EncodedJSValue ObjCTypeScriptExtendFunction(ExecState* execState) {
 
     return JSValue::encode(jsUndefined());
 }
-}
+}

src/NativeScript/ObjC/TNSDataAdapter.mm

@@ -56,8 +56,8 @@ - (void*)mutableBytes {
 - (NSUInteger)length {
     RELEASE_ASSERT_WITH_MESSAGE([TNSRuntime runtimeForVM:self->_vm], "The runtime is deallocated.");
     VM& vm = self->_execState->vm();
-    auto scope = DECLARE_CATCH_SCOPE(vm);
     JSLockHolder lock(self->_execState);
+    auto scope = DECLARE_CATCH_SCOPE(vm);
     NSUInteger length = self->_object->get(self->_execState, self->_execState->propertyNames().byteLength).toUInt32(self->_execState);
     reportErrorIfAny(self->_execState, scope);
     return length;

src/NativeScript/TNSRuntime+Inspector.mm

@@ -57,6 +57,8 @@ - (void)setup;
 @implementation TNSRuntime (Inspector)
 
 - (TNSRuntimeInspector*)attachInspectorWithHandler:(TNSRuntimeInspectorMessageHandler)messageHandler {
+    JSC::JSLockHolder lock(self->_vm.get());
+
     TNSRuntimeInspector* runtimeInspector = [[TNSRuntimeInspector alloc] initWithRuntime:self
                                                                           messageHandler:messageHandler];
 
@@ -133,6 +135,8 @@ - (void)pause {
 }
 
 - (void)dealloc {
+    JSC::JSLockHolder lock(_runtime->_vm.get());
+
     self->_inspectorController->disconnectFrontend(_frontendChannel.get());
     [self->_runtime release];
     [super dealloc];

src/NativeScript/inspector/DomainBackendDispatcher.cpp

@@ -2,9 +2,9 @@
 #include "GlobalObjectInspectorController.h"
 #include "SuppressAllPauses.h"
 #include <JavaScriptCore/Completion.h>
+#include <JavaScriptCore/InspectorAgentBase.h>
 #include <JavaScriptCore/inspector/InspectorBackendDispatchers.h>
 #include <JavaScriptCore/inspector/InspectorFrontendDispatchers.h>
-#include <JavaScriptCore/InspectorAgentBase.h>
 #include <JavaScriptCore/runtime/Exception.h>
 #include <JavaScriptCore/runtime/JSONObject.h>
 #include <stdio.h>
@@ -29,7 +29,7 @@ DomainBackendDispatcher::DomainBackendDispatcher(WTF::String domain, JSCell* con
     const HashMap<String, SupplementalBackendDispatcher*>& dispatchers = m_backendDispatcher->dispatchers();
     auto result = dispatchers.find(domain);
     if (result != dispatchers.end()) {
-        m_duplicatedDispatcher = adoptRef(*result->value);
+        m_duplicatedDispatcher = result->value;
     }
     m_backendDispatcher->registerDispatcherForDomain(domain, this);
 }
@@ -88,4 +88,4 @@ void DomainBackendDispatcher::dispatch(long callId, const String& method, Ref<In
 
     m_backendDispatcher->sendResponse(callId, WTFMove(messageObject));
 }
-}
+} // namespace NativeScript

src/NativeScript/inspector/DomainInspectorAgent.cpp

@@ -6,17 +6,18 @@ namespace NativeScript {
 DomainInspectorAgent::DomainInspectorAgent(WTF::String domainName, JSC::JSCell* constructorFunction, Inspector::JSAgentContext& context)
     : Inspector::InspectorAgentBase(domainName)
     , m_context(context)
-    , m_constructorFunction(m_context.inspectedGlobalObject.vm(), constructorFunction) {}
+    , m_constructorFunction(m_context.inspectedGlobalObject.vm(), constructorFunction) {
 
-void DomainInspectorAgent::didCreateFrontendAndBackend(Inspector::FrontendRouter*, Inspector::BackendDispatcher* backendDispatcher) {
     this->m_domainBackendDispatcher = DomainBackendDispatcher::create(this->domainName(), m_constructorFunction.get(), m_context);
 }
 
+void DomainInspectorAgent::didCreateFrontendAndBackend(Inspector::FrontendRouter*, Inspector::BackendDispatcher* backendDispatcher) {
+}
+
 void DomainInspectorAgent::willDestroyFrontendAndBackend(Inspector::DisconnectReason) {
-    m_domainBackendDispatcher = nullptr;
 }
 
 void DomainInspectorAgent::discardAgent() {
     m_constructorFunction.clear();
 }
-}
+} // namespace NativeScript

src/NativeScript/inspector/InspectorLogAgent.cpp

@@ -64,11 +64,12 @@ InspectorLogAgent::InspectorLogAgent(Inspector::JSAgentContext& context)
     : InspectorAgentBase(ASCIILiteral("Log"))
     , m_injectedScriptManager(context.injectedScriptManager)
     , m_globalObject(*JSC::jsCast<NativeScript::GlobalObject*>(&context.inspectedGlobalObject)) {
+
+    this->m_frontendDispatcher = std::make_unique<LogFrontendDispatcher>(context.frontendRouter);
+    this->m_backendDispatcher = LogBackendDispatcher::create(context.backendDispatcher, this);
 }
 
 void InspectorLogAgent::didCreateFrontendAndBackend(FrontendRouter* frontendDispatcher, BackendDispatcher* backendDispatcher) {
-    this->m_frontendDispatcher = std::make_unique<LogFrontendDispatcher>(*frontendDispatcher);
-    this->m_backendDispatcher = LogBackendDispatcher::create(*backendDispatcher, this);
 }
 
 void InspectorLogAgent::willDestroyFrontendAndBackend(DisconnectReason) {
@@ -153,4 +154,4 @@ void InspectorLogAgent::addMessageToFrontend(ConsoleMessage* consoleMessage) {
     }
     m_frontendDispatcher->entryAdded(WTFMove(jsonObj));
 }
-}
+} // namespace Inspector

src/NativeScript/inspector/InspectorNetworkAgent.cpp

@@ -5,11 +5,11 @@ namespace Inspector {
 InspectorNetworkAgent::InspectorNetworkAgent(JSAgentContext& context)
     : Inspector::InspectorAgentBase(ASCIILiteral("Network"))
     , m_globalObject(*JSC::jsCast<NativeScript::GlobalObject*>(&context.inspectedGlobalObject)) {
+    this->m_frontendDispatcher = std::make_unique<NetworkFrontendDispatcher>(context.frontendRouter);
+    this->m_backendDispatcher = NetworkBackendDispatcher::create(context.backendDispatcher, this);
 }
 
 void InspectorNetworkAgent::didCreateFrontendAndBackend(Inspector::FrontendRouter* frontendRouter, Inspector::BackendDispatcher* backendDispatcher) {
-    m_frontendDispatcher = std::make_unique<NetworkFrontendDispatcher>(*frontendRouter);
-    m_backendDispatcher = NetworkBackendDispatcher::create(*backendDispatcher, this);
 }
 
 void InspectorNetworkAgent::willDestroyFrontendAndBackend(Inspector::DisconnectReason) {
@@ -40,4 +40,4 @@ void InspectorNetworkAgent::loadResource(ErrorString& errorString, const String&
         callback->sendSuccess(content, resource.mimeType(), 200);
     }
 }
-}
+} // namespace Inspector

src/NativeScript/inspector/InspectorPageAgent.mm

@@ -14,16 +14,15 @@
     , m_frameIdentifier("NativeScriptMainFrameIdentifier")
     , m_frameUrl("http://main.xml")
     , m_globalObject(*JSC::jsCast<NativeScript::GlobalObject*>(&context.inspectedGlobalObject)) {
+
+    this->m_frontendDispatcher = std::make_unique<PageFrontendDispatcher>(context.frontendRouter);
+    this->m_backendDispatcher = PageBackendDispatcher::create(context.backendDispatcher, this);
 }
 
 void InspectorPageAgent::didCreateFrontendAndBackend(FrontendRouter* frontendRouter, BackendDispatcher* backendDispatcher) {
-    m_frontendDispatcher = std::make_unique<PageFrontendDispatcher>(*frontendRouter);
-    m_backendDispatcher = PageBackendDispatcher::create(*backendDispatcher, this);
 }
 
 void InspectorPageAgent::willDestroyFrontendAndBackend(DisconnectReason) {
-    m_frontendDispatcher = nullptr;
-    m_backendDispatcher = nullptr;
 }
 
 void InspectorPageAgent::enable(ErrorString&) {

src/NativeScript/inspector/InspectorTimelineAgent.cpp

@@ -17,6 +17,9 @@ InspectorTimelineAgent::InspectorTimelineAgent(JSAgentContext& context, Inspecto
     , m_consoleRecordEntry()
     , m_maxCallStackDepth(5)
     , m_enabled(false) {
+
+    this->m_frontendDispatcher = std::make_unique<TimelineFrontendDispatcher>(context.frontendRouter);
+    this->m_backendDispatcher = TimelineBackendDispatcher::create(context.backendDispatcher, this);
 }
 
 void InspectorTimelineAgent::sendEvent(RefPtr<InspectorObject>&& event) {
@@ -29,15 +32,10 @@ void InspectorTimelineAgent::sendEvent(RefPtr<InspectorObject>&& event) {
 }
 
 void InspectorTimelineAgent::didCreateFrontendAndBackend(FrontendRouter* frontendRouter, BackendDispatcher* backendDispatcher) {
-    m_frontendDispatcher = std::make_unique<TimelineFrontendDispatcher>(*frontendRouter);
-    m_backendDispatcher = TimelineBackendDispatcher::create(*backendDispatcher, this);
-
     this->m_globalObject.inspectorController().setTimelineAgent(this);
 }
 
 void InspectorTimelineAgent::willDestroyFrontendAndBackend(DisconnectReason) {
-    m_frontendDispatcher = nullptr;
-    m_backendDispatcher = nullptr;
 
     ErrorString unused;
     stop(unused);
@@ -327,4 +325,4 @@ void InspectorTimelineAgent::addRecordToTimeline(RefPtr<InspectorObject>&& recor
     auto recordObject = BindingTraits<Inspector::Protocol::Timeline::TimelineEvent>::runtimeCast(WTFMove(record));
     sendEvent(WTFMove(recordObject));
 }
-}
+} // namespace Inspector