From ebe6a114ca0deb89da17ad3f5f78033d2f4d1237 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Mon, 19 Oct 2026 13:36:11 -0300 Subject: [PATCH 1/3] fix(inspector): release the frontend's resources when DevTools disconnects Since #390 closes the client channel on EOF, nothing told JsV8InspectorClient: sender_ kept the closed channel, so every later CDP message (console.log, scriptParsed, ...) failed with "InspectorServer::Send error: Operation canceled". The departed frontend's session also stayed alive with its breakpoints and enabled domains, so a pause after disconnect (or a disconnect while paused) left the app frozen with nobody to resume it. InspectorServer: - Track each client as a Connection with an id and its own serial queue. Closing is idempotent and reports onClientDisconnected(id); the sender drops messages once closed, and ECANCELED from writes queued before the close is no longer logged. - Replace the self-referencing __block read handler (an ARC retain cycle that leaked the block and channel per connection) with a read loop that accumulates each frame until done. - Set SO_NOSIGPIPE on client sockets. JsV8InspectorClient: - On disconnect, clear the sender (ignoring stale connection ids) and reset the frontend's state: IO streams, tracing, worker sessions, and the main session. - Reset the main session through the message queue instead of taking the Locker on the socket thread, which blocked while the isolate was paused. Inside the pause loop the reset resumes and skips further pauses, and the session is swapped by the next drain once the loop has unwound. - Only reset on connect when replacing a still-attached frontend. - Open the listening socket once: the CLI re-posts AttachRequest every few seconds until it sees the port, and each post used to open another listener on the next port. --- NativeScript/inspector/InspectorServer.h | 15 +- NativeScript/inspector/InspectorServer.mm | 230 +++++++++++------- NativeScript/inspector/JsV8InspectorClient.h | 29 ++- NativeScript/inspector/JsV8InspectorClient.mm | 140 ++++++++--- 4 files changed, 292 insertions(+), 122 deletions(-) diff --git a/NativeScript/inspector/InspectorServer.h b/NativeScript/inspector/InspectorServer.h index ef5959157..27b32ebe7 100644 --- a/NativeScript/inspector/InspectorServer.h +++ b/NativeScript/inspector/InspectorServer.h @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -11,14 +12,18 @@ namespace v8_inspector { class InspectorServer { public: + using Sender = std::function; + + // Starts listening and returns the port. Each accepted client gets a + // process-unique connection id: onClientConnected hands out its sender, + // and onClientDisconnected fires once with the same id when the + // connection closes, after which that sender drops every message. + // Callbacks run on the connection's own serial queue. static in_port_t Init( - std::function)> + std::function onClientConnected, + std::function onClientDisconnected, std::function onMessage); - - private: - static void Send(dispatch_io_t channel, dispatch_queue_t queue, - const std::string& message); }; } // namespace v8_inspector diff --git a/NativeScript/inspector/InspectorServer.mm b/NativeScript/inspector/InspectorServer.mm index c18b8758f..cd6e2fd69 100644 --- a/NativeScript/inspector/InspectorServer.mm +++ b/NativeScript/inspector/InspectorServer.mm @@ -3,10 +3,124 @@ #include #include +#include +#include + namespace v8_inspector { +namespace { + +std::atomic gLastConnectionId{0}; + +struct Connection { + uint64_t id; + dispatch_queue_t queue; + dispatch_io_t channel; + std::atomic open{true}; + std::function onDisconnected; + std::function onMessage; +}; + +void CloseConnection(const std::shared_ptr& connection) { + if (!connection->open.exchange(false)) { + return; + } + dispatch_io_close(connection->channel, DISPATCH_IO_STOP); + connection->onDisconnected(connection->id); +} + +// The functions below take the connection by value: a block captures a C++ +// reference as a reference, which would dangle once the caller returns. + +// Calls onComplete with exactly `length` bytes, or closes the connection if +// the peer goes away (or errors) first. +void ReadExactly(std::shared_ptr connection, size_t length, + void (^onComplete)(dispatch_data_t data)) { + __block dispatch_data_t received = dispatch_data_empty; + dispatch_io_read(connection->channel, 0, length, connection->queue, + ^(bool done, dispatch_data_t data, int error) { + if (data != nullptr && dispatch_data_get_size(data) > 0) { + received = dispatch_data_create_concat(received, data); + } + if (!done) { + return; + } + if (error != 0 || dispatch_data_get_size(received) < length) { + if (error != 0 && error != ECANCELED) { + NSLog(@"InspectorServer read error: %s", strerror(error)); + } + CloseConnection(connection); + return; + } + onComplete(received); + }); +} + +// Messages are a big-endian uint32 byte length followed by UTF-16LE text. +void ReadNextMessage(std::shared_ptr connection) { + ReadExactly(connection, sizeof(uint32_t), ^(dispatch_data_t header) { + uint32_t length = 0; + [(NSData*)header getBytes:&length length:sizeof(length)]; + length = ntohl(length); + if (length == 0) { + CloseConnection(connection); + return; + } + + ReadExactly(connection, length, ^(dispatch_data_t body) { + NSString* payload = [[NSString alloc] initWithData:(NSData*)body + encoding:NSUTF16LittleEndianStringEncoding]; + const char* utf8 = [payload UTF8String]; + if (utf8 != nullptr) { + connection->onMessage(utf8); + } + ReadNextMessage(connection); + }); + }); +} + +void Send(std::shared_ptr connection, const std::string& message) { + if (!connection->open.load()) { + return; + } + + NSString* str = [NSString stringWithUTF8String:message.c_str()]; + NSUInteger length = [str lengthOfBytesUsingEncoding:NSUTF16LittleEndianStringEncoding]; + + uint8_t* buffer = (uint8_t*)malloc(length + sizeof(uint32_t)); + + *(uint32_t*)buffer = htonl(length); + + [str getBytes:&buffer[sizeof(uint32_t)] + maxLength:length + usedLength:NULL + encoding:NSUTF16LittleEndianStringEncoding + options:0 + range:NSMakeRange(0, str.length) + remainingRange:NULL]; + + dispatch_data_t data = + dispatch_data_create(buffer, length + sizeof(uint32_t), connection->queue, ^{ + free(buffer); + }); + + dispatch_io_write(connection->channel, 0, data, connection->queue, + ^(bool done, dispatch_data_t data, int error) { + // ECANCELED: the write was still queued when the + // connection closed. + if (error == 0 || error == ECANCELED) { + return; + } + NSLog(@"InspectorServer::Send error: %s", strerror(error)); + CloseConnection(connection); + }); +} + +} // namespace + in_port_t InspectorServer::Init( - std::function)> onClientConnected, + std::function onClientConnected, + std::function onClientDisconnected, std::function onMessage) { in_port_t listenPort = 18183; @@ -43,68 +157,40 @@ unsigned int clntLen = sizeof(echoClntAddr); // Wait for a client to connect - if ((clientSocket = accept(serverSocket, (struct sockaddr*)&echoClntAddr, &clntLen)) >= 0) { - dispatch_io_t channel = - dispatch_io_create(DISPATCH_IO_STREAM, clientSocket, q, ^(int error) { - if (error) { - NSLog(@"InspectorServer channel cleanup error: %s", strerror(error)); - } - close(clientSocket); - }); - - onClientConnected([channel, q](std::string message) { Send(channel, q, message); }); - - __block dispatch_io_handler_t receiver = ^(bool done, dispatch_data_t data, int error) { - if (error) { - NSLog(@"InspectorServer read message header error: %s", strerror(error)); - } - - const void* bytes = [(NSData*)data bytes]; - if (!bytes) { - dispatch_io_close(channel, DISPATCH_IO_STOP); - return; - } - - uint32_t length = ntohl(*(uint32_t*)bytes); - - // Configure the channel... - dispatch_io_set_low_water(channel, length); - - // Setup read handler - dispatch_io_read(channel, 0, length, q, ^(bool done, dispatch_data_t data, int error) { - BOOL close = NO; - if (error) { - NSLog(@"InspectorServer read message body error: %s", strerror(error)); - close = YES; - } - - const size_t size = data ? dispatch_data_get_size(data) : 0; - if (size) { - NSString* payload = [[NSString alloc] initWithData:(NSData*)data - encoding:NSUTF16LittleEndianStringEncoding]; - - onMessage([payload UTF8String]); - -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Warc-retain-cycles" - dispatch_io_read(channel, 0, 4, q, receiver); -#pragma clang diagnostic pop - } else { - close = YES; - } + if ((clientSocket = accept(serverSocket, (struct sockaddr*)&echoClntAddr, &clntLen)) < 0) { + NSLog(@"InspectorServer accept() failed"); + continue; + } - if (close) { - dispatch_io_close(channel, DISPATCH_IO_STOP); + // A write racing the peer's close must fail with EPIPE, not kill the app. + int noSigPipe = 1; + setsockopt(clientSocket, SOL_SOCKET, SO_NOSIGPIPE, &noSigPipe, sizeof(noSigPipe)); + + auto connection = std::make_shared(); + connection->id = ++gLastConnectionId; + // Serial, so the read handlers that assemble one message never overlap. + connection->queue = + dispatch_queue_create("NativeScript.v8.inspector.connection", DISPATCH_QUEUE_SERIAL); + connection->onDisconnected = onClientDisconnected; + connection->onMessage = onMessage; + connection->channel = + dispatch_io_create(DISPATCH_IO_STREAM, clientSocket, connection->queue, ^(int error) { + if (error) { + NSLog(@"InspectorServer channel cleanup error: %s", strerror(error)); } + close(clientSocket); }); - }; - if (channel) { - dispatch_io_read(channel, 0, 4, q, receiver); - } - } else { - NSLog(@"InspectorServer accept() failed"); + if (connection->channel == nullptr) { + close(clientSocket); + continue; } + + dispatch_async(connection->queue, ^{ + onClientConnected(connection->id, + [connection](const std::string& message) { Send(connection, message); }); + ReadNextMessage(connection); + }); } }); @@ -121,32 +207,4 @@ return listenPort; } -void InspectorServer::Send(dispatch_io_t channel, dispatch_queue_t queue, - const std::string& message) { - NSString* str = [NSString stringWithUTF8String:message.c_str()]; - NSUInteger length = [str lengthOfBytesUsingEncoding:NSUTF16LittleEndianStringEncoding]; - - uint8_t* buffer = (uint8_t*)malloc(length + sizeof(uint32_t)); - - *(uint32_t*)buffer = htonl(length); - - [str getBytes:&buffer[sizeof(uint32_t)] - maxLength:length - usedLength:NULL - encoding:NSUTF16LittleEndianStringEncoding - options:0 - range:NSMakeRange(0, str.length) - remainingRange:NULL]; - - dispatch_data_t data = dispatch_data_create(buffer, length + sizeof(uint32_t), queue, ^{ - free(buffer); - }); - - dispatch_io_write(channel, 0, data, queue, ^(bool done, dispatch_data_t data, int error) { - if (error) { - NSLog(@"InspectorServer::Send error: %s", strerror(error)); - } - }); -} - } // namespace v8_inspector diff --git a/NativeScript/inspector/JsV8InspectorClient.h b/NativeScript/inspector/JsV8InspectorClient.h index 5b88c9c4c..768dfdf20 100644 --- a/NativeScript/inspector/JsV8InspectorClient.h +++ b/NativeScript/inspector/JsV8InspectorClient.h @@ -2,7 +2,9 @@ #define JsV8InspectorClient_h #include +#include +#include #include #include #include @@ -23,7 +25,6 @@ class JsV8InspectorClient : V8InspectorClient, V8Inspector::Channel { JsV8InspectorClient(tns::Runtime* runtime); void init(); void connect(int argc, char** argv); - void disconnect(); void dispatchMessage(const std::string& message); // The single instance debugging the main isolate (created when IsDebug); @@ -64,7 +65,17 @@ class JsV8InspectorClient : V8InspectorClient, V8Inspector::Channel { private: static constexpr int contextGroupId = 1; - bool isConnected_; + // Queued behind the departing frontend's messages; replaces the main + // session when it reaches the front of the queue. + static constexpr const char* kResetSessionMessage = + "{\"__nsInternal\":\"resetSession\"}"; + + std::atomic isConnected_{false}; + // Set when a reset arrives inside the pause loop (which may run on any + // thread holding the Locker), where the session may still be on the stack; + // the next drain after the loop exits performs it. + std::atomic pendingSessionReset_{false}; + in_port_t listenPort_ = 0; // main queue only std::unique_ptr inspector_; v8::Persistent context_; std::unique_ptr session_; @@ -77,6 +88,8 @@ class JsV8InspectorClient : V8InspectorClient, V8Inspector::Channel { dispatch_queue_t messageLoopQueue_; dispatch_semaphore_t messageArrived_; std::function sender_; + uint64_t connectionId_ = + 0; // id of sender_'s connection, guarded by senderMutex_ bool isWaitingForDebugger_; bool hasScheduledDebugBreak_; @@ -126,8 +139,18 @@ class JsV8InspectorClient : V8InspectorClient, V8Inspector::Channel { void createInspectorSession(); void notify(std::unique_ptr message); void notify(const std::string& message); - void onFrontendConnected(std::function sender); + void onFrontendConnected(uint64_t connectionId, + std::function sender); + void onFrontendDisconnected(uint64_t connectionId); void onFrontendMessageReceived(const std::string& message); + // Socket thread: drops the state owned by the departing frontend that does + // not need the main isolate (IO streams, tracing, worker sessions). + void ResetFrontendState(); + // Pause loop only: resumes and defers the session swap to the next drain. + void DeferSessionResetFromPauseLoop(); + void ResetMainSession(); + void EnqueueMessage(const std::string& message); + void ScheduleMessageDrain(); std::string PumpMessage(); static void registerDomainDispatcherCallback( const v8::FunctionCallbackInfo& args); diff --git a/NativeScript/inspector/JsV8InspectorClient.mm b/NativeScript/inspector/JsV8InspectorClient.mm index 7af9b0c67..fe7900096 100644 --- a/NativeScript/inspector/JsV8InspectorClient.mm +++ b/NativeScript/inspector/JsV8InspectorClient.mm @@ -169,13 +169,18 @@ bool ShouldRewriteSourceMapURLs() { notify_register_dispatch( NOTIFICATION("AttachRequest"), &attachRequestSubscription, dispatch_get_main_queue(), ^(int token) { - in_port_t listenPort = InspectorServer::Init( - [this](std::function sender) { - this->onFrontendConnected(sender); - }, - [this](const std::string& message) { this->onFrontendMessageReceived(message); }); + // The CLI re-posts AttachRequest (every few seconds until it sees the + // port, and again per debug session); answer each with the same port. + if (this->listenPort_ == 0) { + this->listenPort_ = InspectorServer::Init( + [this](uint64_t connectionId, std::function sender) { + this->onFrontendConnected(connectionId, sender); + }, + [this](uint64_t connectionId) { this->onFrontendDisconnected(connectionId); }, + [this](const std::string& message) { this->onFrontendMessageReceived(message); }); + } - LOG_DEBUGGER_PORT(listenPort); + LOG_DEBUGGER_PORT(this->listenPort_); notify_post(NOTIFICATION("ReadyForAttach")); }); @@ -205,7 +210,8 @@ bool ShouldRewriteSourceMapURLs() { notify_cancel(waitForDebuggerSubscription); } -void JsV8InspectorClient::onFrontendConnected(std::function sender) { +void JsV8InspectorClient::onFrontendConnected(uint64_t connectionId, + std::function sender) { if (this->isWaitingForDebugger_) { this->isWaitingForDebugger_ = NO; CFRunLoopRef runloop = CFRunLoopGetMain(); @@ -216,17 +222,45 @@ bool ShouldRewriteSourceMapURLs() { CFRunLoopWakeUp(runloop); } + bool replacesFrontend; { std::lock_guard lock(this->senderMutex_); + replacesFrontend = this->connectionId_ != 0; this->sender_ = sender; + this->connectionId_ = connectionId; } - // this triggers a reconnection from the devtools so Debugger.scriptParsed etc. are all fired - // again - this->disconnect(); + // The frontend's Debugger.enable must land on a fresh session so it replays + // Debugger.scriptParsed etc. Without a previous frontend the session is + // either untouched or its reset is already queued by the disconnect. + // Otherwise the reset is queued ahead of anything this connection sends, + // since its reads only start after this returns. + if (replacesFrontend) { + this->ResetFrontendState(); + this->EnqueueMessage(kResetSessionMessage); + this->ScheduleMessageDrain(); + } this->isConnected_ = true; } +void JsV8InspectorClient::onFrontendDisconnected(uint64_t connectionId) { + { + std::lock_guard lock(this->senderMutex_); + // A newer connection already took over. + if (this->connectionId_ != connectionId) { + return; + } + this->sender_ = nullptr; + this->connectionId_ = 0; + } + + // Nobody is left to resume a pause or read what the session emits. + this->isConnected_ = false; + this->ResetFrontendState(); + this->EnqueueMessage(kResetSessionMessage); + this->ScheduleMessageDrain(); +} + void JsV8InspectorClient::onFrontendMessageReceived(const std::string& message) { // Single parse, on the socket thread, used for routing and fast paths. auto parsed = json::parse(message, nullptr, false); @@ -354,10 +388,7 @@ bool ShouldRewriteSourceMapURLs() { return; } - dispatch_sync(this->messagesQueue_, ^{ - this->messages_.push(message); - dispatch_semaphore_signal(messageArrived_); - }); + this->EnqueueMessage(message); // Debugger.pause needs to interrupt V8 even if the main thread is busy // executing JS. RequestInterrupt fires at the next safe bytecode boundary. @@ -370,12 +401,32 @@ bool ShouldRewriteSourceMapURLs() { this); } - tns::ExecuteOnMainThread([this, message]() { + this->ScheduleMessageDrain(); +} + +void JsV8InspectorClient::EnqueueMessage(const std::string& message) { + dispatch_sync(this->messagesQueue_, ^{ + this->messages_.push(message); + dispatch_semaphore_signal(messageArrived_); + }); +} + +void JsV8InspectorClient::ScheduleMessageDrain() { + tns::ExecuteOnMainThread([this]() { dispatch_sync(this->messageLoopQueue_, ^{ // prevent execution if we're already pumping messages if (runningNestedLoops_ && !terminated_) { return; }; + if (this->pendingSessionReset_) { + // A pause loop (possibly on another thread) is still winding down; + // it schedules another drain once it has exited. + if (runningNestedLoops_) { + return; + } + this->pendingSessionReset_ = false; + this->ResetMainSession(); + } std::string message; do { message = this->PumpMessage(); @@ -448,7 +499,7 @@ bool ShouldRewriteSourceMapURLs() { v8_inspector::V8Inspector::kFullyTrusted); } -void JsV8InspectorClient::disconnect() { +void JsV8InspectorClient::ResetFrontendState() { // Resource stream handles only have meaning to the frontend that opened // them, so drop any streams it never closed via IO.close. { @@ -460,18 +511,6 @@ bool ShouldRewriteSourceMapURLs() { // controller is free for the next frontend's Tracing.start. tracing_agent_->stopAndDiscard(); - Isolate* isolate = isolate_; - v8::Locker locker(isolate); - Isolate::Scope isolate_scope(isolate); - HandleScope handle_scope(isolate); - - session_->resume(); - session_.reset(); - - this->isConnected_ = false; - - this->createInspectorSession(); - // Reset worker sessions too: resume any paused worker and recreate its // session so the (re)connecting frontend gets a clean slate, then forget // the auto-attach state until it sends Target.setAutoAttach again. @@ -485,7 +524,38 @@ bool ShouldRewriteSourceMapURLs() { } } +void JsV8InspectorClient::DeferSessionResetFromPauseLoop() { + // The pause loop may itself run under session_->dispatchProtocolMessage, + // and V8 touches every session right after the loop returns, so the swap + // must wait until the stack has unwound. Resume now and keep the old + // session from pausing again in the meantime. + this->pendingSessionReset_ = true; + Isolate* isolate = isolate_; + v8::Locker locker(isolate); + Isolate::Scope isolate_scope(isolate); + HandleScope handle_scope(isolate); + session_->setSkipAllPauses(true); + session_->resume(); +} + +void JsV8InspectorClient::ResetMainSession() { + Isolate* isolate = isolate_; + v8::Locker locker(isolate); + Isolate::Scope isolate_scope(isolate); + HandleScope handle_scope(isolate); + + session_->resume(); + session_.reset(); + + this->createInspectorSession(); +} + void JsV8InspectorClient::runMessageLoopOnPause(int contextGroupId) { + // The session that paused is about to be replaced; no frontend owns it. + if (this->pendingSessionReset_) { + return; + } + __block auto loopsRunning = false; dispatch_sync(this->messageLoopQueue_, ^{ loopsRunning = runningNestedLoops_; @@ -503,6 +573,10 @@ bool ShouldRewriteSourceMapURLs() { bool shouldWait = false; while (!terminated_) { std::string message = this->PumpMessage(); + if (message == kResetSessionMessage) { + this->DeferSessionResetFromPauseLoop(); + break; + } if (!message.empty()) { this->dispatchMessage(message); shouldWait = false; @@ -528,6 +602,10 @@ bool ShouldRewriteSourceMapURLs() { terminated_ = false; runningNestedLoops_ = false; }); + + if (this->pendingSessionReset_) { + this->ScheduleMessageDrain(); + } } void JsV8InspectorClient::quitMessageLoopOnPause() { @@ -569,6 +647,12 @@ bool ShouldRewriteSourceMapURLs() { } void JsV8InspectorClient::dispatchMessage(const std::string& message) { + // The pause loop intercepts this message, so here no session is on the stack. + if (message == kResetSessionMessage) { + this->ResetMainSession(); + return; + } + auto json_message = json::parse(message); std::string method = json_message["method"]; From 4da80ace5cf7c06cf6139c506a6db1b58d75e466 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Mon, 19 Oct 2026 13:51:17 -0300 Subject: [PATCH 2/3] fix(inspector): serialize frontend connect/disconnect, one client at a time Each connection runs its callbacks on its own queue, so an old connection's disconnect could interleave with the next connection's connect: the new frontend saw no attached frontend, skipped its reset, and its Debugger.enable / Tracing.start / IO streams were then wiped by the old disconnect's late cleanup. Serialize both handlers with a dedicated lifecycle mutex (not senderMutex_, which RegisterWorkerTarget takes after workerTargetsMutex_). The server now accepts one client at a time: a new client closes the previous one, under a lock held across both callbacks so back-to-back accepts cannot hand out senders in the opposite order. A session reset also drops the departing frontend's still-queued messages, so their replies (whose ids may collide) never reach the next frontend. --- NativeScript/inspector/InspectorServer.h | 4 ++- NativeScript/inspector/InspectorServer.mm | 22 +++++++++++++-- NativeScript/inspector/JsV8InspectorClient.h | 5 ++++ NativeScript/inspector/JsV8InspectorClient.mm | 27 ++++++++++++++----- 4 files changed, 49 insertions(+), 9 deletions(-) diff --git a/NativeScript/inspector/InspectorServer.h b/NativeScript/inspector/InspectorServer.h index 27b32ebe7..93711aaac 100644 --- a/NativeScript/inspector/InspectorServer.h +++ b/NativeScript/inspector/InspectorServer.h @@ -18,7 +18,9 @@ class InspectorServer { // process-unique connection id: onClientConnected hands out its sender, // and onClientDisconnected fires once with the same id when the // connection closes, after which that sender drops every message. - // Callbacks run on the connection's own serial queue. + // One client at a time: accepting a client closes the previous one. The + // previous client's onClientDisconnected may still be running on its own + // queue when the new onClientConnected starts. static in_port_t Init( std::function onClientConnected, diff --git a/NativeScript/inspector/InspectorServer.mm b/NativeScript/inspector/InspectorServer.mm index cd6e2fd69..296c3e6b7 100644 --- a/NativeScript/inspector/InspectorServer.mm +++ b/NativeScript/inspector/InspectorServer.mm @@ -5,6 +5,7 @@ #include #include +#include namespace v8_inspector { @@ -145,6 +146,12 @@ void Send(std::shared_ptr connection, const std::string& message) { assert(false); } + struct ActiveConnection { + std::mutex mutex; + std::weak_ptr connection; + }; + auto active = std::make_shared(); + // Set up the dispatch source that will alert us to new incoming connections dispatch_queue_t q = dispatch_queue_create("server_queue", DISPATCH_QUEUE_CONCURRENT); dispatch_source_t acceptSource = @@ -187,8 +194,19 @@ void Send(std::shared_ptr connection, const std::string& message) { } dispatch_async(connection->queue, ^{ - onClientConnected(connection->id, - [connection](const std::string& message) { Send(connection, message); }); + { + // Held across both callbacks so two clients accepted back to back + // cannot hand out their senders in the opposite order. + std::lock_guard lock(active->mutex); + std::shared_ptr previous = active->connection.lock(); + active->connection = connection; + if (previous != nullptr) { + CloseConnection(previous); + } + onClientConnected(connection->id, [connection](const std::string& message) { + Send(connection, message); + }); + } ReadNextMessage(connection); }); } diff --git a/NativeScript/inspector/JsV8InspectorClient.h b/NativeScript/inspector/JsV8InspectorClient.h index 768dfdf20..cfee5a221 100644 --- a/NativeScript/inspector/JsV8InspectorClient.h +++ b/NativeScript/inspector/JsV8InspectorClient.h @@ -123,6 +123,10 @@ class JsV8InspectorClient : V8InspectorClient, V8Inspector::Channel { static JsV8InspectorClient* instance_; std::mutex senderMutex_; + // Serializes onFrontendConnected/onFrontendDisconnected, which run on + // different connection queues. Taken before senderMutex_ and + // workerTargetsMutex_, never while holding either. + std::mutex frontendLifecycleMutex_; // Routes a frontend message carrying a sessionId to its worker session // (socket thread). msgId is -1 when the message has no id. @@ -149,6 +153,7 @@ class JsV8InspectorClient : V8InspectorClient, V8Inspector::Channel { // Pause loop only: resumes and defers the session swap to the next drain. void DeferSessionResetFromPauseLoop(); void ResetMainSession(); + void QueueSessionReset(); void EnqueueMessage(const std::string& message); void ScheduleMessageDrain(); std::string PumpMessage(); diff --git a/NativeScript/inspector/JsV8InspectorClient.mm b/NativeScript/inspector/JsV8InspectorClient.mm index fe7900096..ce5e80d84 100644 --- a/NativeScript/inspector/JsV8InspectorClient.mm +++ b/NativeScript/inspector/JsV8InspectorClient.mm @@ -212,6 +212,8 @@ bool ShouldRewriteSourceMapURLs() { void JsV8InspectorClient::onFrontendConnected(uint64_t connectionId, std::function sender) { + std::lock_guard lifecycle(this->frontendLifecycleMutex_); + if (this->isWaitingForDebugger_) { this->isWaitingForDebugger_ = NO; CFRunLoopRef runloop = CFRunLoopGetMain(); @@ -232,18 +234,20 @@ bool ShouldRewriteSourceMapURLs() { // The frontend's Debugger.enable must land on a fresh session so it replays // Debugger.scriptParsed etc. Without a previous frontend the session is - // either untouched or its reset is already queued by the disconnect. - // Otherwise the reset is queued ahead of anything this connection sends, - // since its reads only start after this returns. + // either untouched or the disconnect (serialized with us by + // frontendLifecycleMutex_) already queued its reset. Either way the reset + // is ahead of anything this connection sends, since its reads only start + // after this returns. if (replacesFrontend) { this->ResetFrontendState(); - this->EnqueueMessage(kResetSessionMessage); - this->ScheduleMessageDrain(); + this->QueueSessionReset(); } this->isConnected_ = true; } void JsV8InspectorClient::onFrontendDisconnected(uint64_t connectionId) { + std::lock_guard lifecycle(this->frontendLifecycleMutex_); + { std::lock_guard lock(this->senderMutex_); // A newer connection already took over. @@ -257,7 +261,18 @@ bool ShouldRewriteSourceMapURLs() { // Nobody is left to resume a pause or read what the session emits. this->isConnected_ = false; this->ResetFrontendState(); - this->EnqueueMessage(kResetSessionMessage); + this->QueueSessionReset(); +} + +void JsV8InspectorClient::QueueSessionReset() { + // Everything still queued came from the departing frontend. Dropping it + // keeps its replies, whose ids may collide with the next frontend's, from + // reaching that frontend. + dispatch_sync(this->messagesQueue_, ^{ + this->messages_ = {}; + this->messages_.push(kResetSessionMessage); + dispatch_semaphore_signal(messageArrived_); + }); this->ScheduleMessageDrain(); } From 32e8ee1671df41842c1af380be66622a9868ea95 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Mon, 19 Oct 2026 17:54:38 -0300 Subject: [PATCH 3/3] fix(inspector): drop messages from a connection after it is replaced Closing a replaced connection does not cancel a read handler that has already assembled a message, so the old frontend's last command could still be dispatched to the new frontend's session, and its reply sent to the new frontend. Message callbacks now carry the connection id, and onFrontendMessageReceived drops messages from a connection that is no longer current. The check and the enqueue run under the lifecycle mutex, so a concurrent takeover's reset drops anything queued before it. --- NativeScript/inspector/InspectorServer.h | 6 ++++-- NativeScript/inspector/InspectorServer.mm | 6 +++--- NativeScript/inspector/JsV8InspectorClient.h | 10 ++++++---- NativeScript/inspector/JsV8InspectorClient.mm | 19 +++++++++++++++++-- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/NativeScript/inspector/InspectorServer.h b/NativeScript/inspector/InspectorServer.h index 93711aaac..cb957c9d3 100644 --- a/NativeScript/inspector/InspectorServer.h +++ b/NativeScript/inspector/InspectorServer.h @@ -20,12 +20,14 @@ class InspectorServer { // connection closes, after which that sender drops every message. // One client at a time: accepting a client closes the previous one. The // previous client's onClientDisconnected may still be running on its own - // queue when the new onClientConnected starts. + // queue when the new onClientConnected starts, and a message it had + // already read may still reach onMessage afterwards, so callers must + // check the connection id. static in_port_t Init( std::function onClientConnected, std::function onClientDisconnected, - std::function onMessage); + std::function onMessage); }; } // namespace v8_inspector diff --git a/NativeScript/inspector/InspectorServer.mm b/NativeScript/inspector/InspectorServer.mm index 296c3e6b7..307ee898c 100644 --- a/NativeScript/inspector/InspectorServer.mm +++ b/NativeScript/inspector/InspectorServer.mm @@ -19,7 +19,7 @@ dispatch_io_t channel; std::atomic open{true}; std::function onDisconnected; - std::function onMessage; + std::function onMessage; }; void CloseConnection(const std::shared_ptr& connection) { @@ -73,7 +73,7 @@ void ReadNextMessage(std::shared_ptr connection) { encoding:NSUTF16LittleEndianStringEncoding]; const char* utf8 = [payload UTF8String]; if (utf8 != nullptr) { - connection->onMessage(utf8); + connection->onMessage(connection->id, utf8); } ReadNextMessage(connection); }); @@ -122,7 +122,7 @@ void Send(std::shared_ptr connection, const std::string& message) { in_port_t InspectorServer::Init( std::function onClientConnected, std::function onClientDisconnected, - std::function onMessage) { + std::function onMessage) { in_port_t listenPort = 18183; int serverSocket = -1; diff --git a/NativeScript/inspector/JsV8InspectorClient.h b/NativeScript/inspector/JsV8InspectorClient.h index cfee5a221..995aec046 100644 --- a/NativeScript/inspector/JsV8InspectorClient.h +++ b/NativeScript/inspector/JsV8InspectorClient.h @@ -123,9 +123,10 @@ class JsV8InspectorClient : V8InspectorClient, V8Inspector::Channel { static JsV8InspectorClient* instance_; std::mutex senderMutex_; - // Serializes onFrontendConnected/onFrontendDisconnected, which run on - // different connection queues. Taken before senderMutex_ and - // workerTargetsMutex_, never while holding either. + // Serializes onFrontendConnected/onFrontendDisconnected/ + // onFrontendMessageReceived, which run on different connection queues. + // Taken before senderMutex_, workerTargetsMutex_ and resourceStreamsMutex_, + // never while holding any of them. std::mutex frontendLifecycleMutex_; // Routes a frontend message carrying a sessionId to its worker session @@ -146,7 +147,8 @@ class JsV8InspectorClient : V8InspectorClient, V8Inspector::Channel { void onFrontendConnected(uint64_t connectionId, std::function sender); void onFrontendDisconnected(uint64_t connectionId); - void onFrontendMessageReceived(const std::string& message); + void onFrontendMessageReceived(uint64_t connectionId, + const std::string& message); // Socket thread: drops the state owned by the departing frontend that does // not need the main isolate (IO streams, tracing, worker sessions). void ResetFrontendState(); diff --git a/NativeScript/inspector/JsV8InspectorClient.mm b/NativeScript/inspector/JsV8InspectorClient.mm index ce5e80d84..fb36b5975 100644 --- a/NativeScript/inspector/JsV8InspectorClient.mm +++ b/NativeScript/inspector/JsV8InspectorClient.mm @@ -177,7 +177,9 @@ bool ShouldRewriteSourceMapURLs() { this->onFrontendConnected(connectionId, sender); }, [this](uint64_t connectionId) { this->onFrontendDisconnected(connectionId); }, - [this](const std::string& message) { this->onFrontendMessageReceived(message); }); + [this](uint64_t connectionId, const std::string& message) { + this->onFrontendMessageReceived(connectionId, message); + }); } LOG_DEBUGGER_PORT(this->listenPort_); @@ -276,7 +278,20 @@ bool ShouldRewriteSourceMapURLs() { this->ScheduleMessageDrain(); } -void JsV8InspectorClient::onFrontendMessageReceived(const std::string& message) { +void JsV8InspectorClient::onFrontendMessageReceived(uint64_t connectionId, + const std::string& message) { + // Held through processing so a takeover cannot run between the check and + // the enqueue: its reset then drops anything this queued. + std::lock_guard lifecycle(this->frontendLifecycleMutex_); + { + std::lock_guard lock(this->senderMutex_); + // Read before its connection was replaced; the reply would reach the new + // frontend. + if (this->connectionId_ != connectionId) { + return; + } + } + // Single parse, on the socket thread, used for routing and fast paths. auto parsed = json::parse(message, nullptr, false); std::string sessionId;