diff --git a/NativeScript/inspector/InspectorServer.h b/NativeScript/inspector/InspectorServer.h index ef595915..cb957c9d 100644 --- a/NativeScript/inspector/InspectorServer.h +++ b/NativeScript/inspector/InspectorServer.h @@ -4,6 +4,7 @@ #include #include +#include #include #include @@ -11,14 +12,22 @@ 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. + // 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, 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)> + std::function onClientConnected, - std::function onMessage); - - private: - static void Send(dispatch_io_t channel, dispatch_queue_t queue, - const std::string& message); + std::function onClientDisconnected, + std::function onMessage); }; } // namespace v8_inspector diff --git a/NativeScript/inspector/InspectorServer.mm b/NativeScript/inspector/InspectorServer.mm index c18b8758..307ee898 100644 --- a/NativeScript/inspector/InspectorServer.mm +++ b/NativeScript/inspector/InspectorServer.mm @@ -3,11 +3,126 @@ #include #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(connection->id, 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 onMessage) { + std::function onClientConnected, + std::function onClientDisconnected, + std::function onMessage) { in_port_t listenPort = 18183; int serverSocket = -1; @@ -31,6 +146,12 @@ 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 = @@ -43,68 +164,51 @@ 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); + if ((clientSocket = accept(serverSocket, (struct sockaddr*)&echoClntAddr, &clntLen)) < 0) { + NSLog(@"InspectorServer accept() failed"); + continue; + } - // Setup read handler - dispatch_io_read(channel, 0, length, q, ^(bool done, dispatch_data_t data, int error) { - BOOL close = NO; + // 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 read message body error: %s", strerror(error)); - close = YES; + NSLog(@"InspectorServer channel cleanup error: %s", strerror(error)); } + close(clientSocket); + }); - 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 (connection->channel == nullptr) { + close(clientSocket); + continue; + } - if (close) { - dispatch_io_close(channel, DISPATCH_IO_STOP); - } + dispatch_async(connection->queue, ^{ + { + // 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); }); - }; - - if (channel) { - dispatch_io_read(channel, 0, 4, q, receiver); } - } else { - NSLog(@"InspectorServer accept() failed"); - } + ReadNextMessage(connection); + }); } }); @@ -121,32 +225,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 5b88c9c4..995aec04 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_; @@ -110,6 +123,11 @@ class JsV8InspectorClient : V8InspectorClient, V8Inspector::Channel { static JsV8InspectorClient* instance_; std::mutex senderMutex_; + // 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 // (socket thread). msgId is -1 when the message has no id. @@ -126,8 +144,20 @@ 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 onFrontendMessageReceived(const std::string& message); + void onFrontendConnected(uint64_t connectionId, + std::function sender); + void onFrontendDisconnected(uint64_t connectionId); + 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(); + // 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(); static void registerDomainDispatcherCallback( const v8::FunctionCallbackInfo& args); diff --git a/NativeScript/inspector/JsV8InspectorClient.mm b/NativeScript/inspector/JsV8InspectorClient.mm index 7af9b0c6..fb36b597 100644 --- a/NativeScript/inspector/JsV8InspectorClient.mm +++ b/NativeScript/inspector/JsV8InspectorClient.mm @@ -169,13 +169,20 @@ 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](uint64_t connectionId, const std::string& message) { + this->onFrontendMessageReceived(connectionId, message); + }); + } - LOG_DEBUGGER_PORT(listenPort); + LOG_DEBUGGER_PORT(this->listenPort_); notify_post(NOTIFICATION("ReadyForAttach")); }); @@ -205,7 +212,10 @@ bool ShouldRewriteSourceMapURLs() { notify_cancel(waitForDebuggerSubscription); } -void JsV8InspectorClient::onFrontendConnected(std::function sender) { +void JsV8InspectorClient::onFrontendConnected(uint64_t connectionId, + std::function sender) { + std::lock_guard lifecycle(this->frontendLifecycleMutex_); + if (this->isWaitingForDebugger_) { this->isWaitingForDebugger_ = NO; CFRunLoopRef runloop = CFRunLoopGetMain(); @@ -216,18 +226,72 @@ 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 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->QueueSessionReset(); + } this->isConnected_ = true; } -void JsV8InspectorClient::onFrontendMessageReceived(const std::string& message) { +void JsV8InspectorClient::onFrontendDisconnected(uint64_t connectionId) { + std::lock_guard lifecycle(this->frontendLifecycleMutex_); + + { + 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->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(); +} + +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; @@ -354,10 +418,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 +431,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 +529,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 +541,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 +554,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 +603,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 +632,10 @@ bool ShouldRewriteSourceMapURLs() { terminated_ = false; runningNestedLoops_ = false; }); + + if (this->pendingSessionReset_) { + this->ScheduleMessageDrain(); + } } void JsV8InspectorClient::quitMessageLoopOnPause() { @@ -569,6 +677,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"];