Skip to content

Commit 600f903

Browse files
authored
fix(inspector): release the frontend's resources when DevTools disconnects (#516)
* 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. * 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. * 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. * fix(inspector): keep a replaced frontend's output away from its successor A command already executing when a new frontend takes over replied through the current sender, so the new frontend received the old one's reply. The main session now has its own binding: all of its output (V8 replies and events, JS-domain replies, __inspectorSendEvent, Target events) goes through notify() to the connection it serves. A sync message, queued on every connect and disconnect ahead of the new frontend's messages, rebinds it on the main thread and replaces the session first if it served another connection. Inside the pause loop only a needed replacement is deferred. Worker sessions get the same guarantee through a frontend generation, bumped atomically with the sender swap whenever a frontend goes away or is replaced. A worker session records it when created, reset or registered, and output with a stale generation is dropped. AnnounceWorkerTargets bails once a frontend change cleared auto-attach, so it cannot mark workers announced that the next frontend never saw, and Target.setAutoAttach from a replaced frontend no longer turns auto-attach on for its successor. Takeover and onClientConnected now run on the accept handler, which is serial, so back-to-back accepts take over in accept order. Also initialize isWaitingForDebugger_, hasScheduledDebugBreak_ and terminated_, which were read before ever being set.
1 parent 20935bd commit 600f903

6 files changed

Lines changed: 472 additions & 135 deletions

File tree

‎NativeScript/inspector/InspectorServer.h‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,31 @@
44
#include <dispatch/dispatch.h>
55
#include <sys/types.h>
66

7+
#include <cstdint>
78
#include <functional>
89
#include <string>
910

1011
namespace v8_inspector {
1112

1213
class InspectorServer {
1314
public:
15+
using Sender = std::function<void(const std::string&)>;
16+
17+
// Starts listening and returns the port. Each accepted client gets a
18+
// process-unique connection id: onClientConnected hands out its sender,
19+
// and onClientDisconnected fires once with the same id when the
20+
// connection closes, after which that sender drops every message.
21+
// One client at a time: accepting a client closes the previous one, and
22+
// that close reports onClientDisconnected before the new onClientConnected.
23+
// A client that closed itself (EOF, read or write error) reports from its
24+
// own queue, which can interleave with a new onClientConnected. A message
25+
// the previous client had already read may reach onMessage after the
26+
// takeover, so callers must check the connection id.
1427
static in_port_t Init(
15-
std::function<void(std::function<void(const std::string&)>)>
28+
std::function<void(uint64_t connectionId, Sender sender)>
1629
onClientConnected,
17-
std::function<void(const std::string&)> onMessage);
18-
19-
private:
20-
static void Send(dispatch_io_t channel, dispatch_queue_t queue,
21-
const std::string& message);
30+
std::function<void(uint64_t connectionId)> onClientDisconnected,
31+
std::function<void(uint64_t connectionId, const std::string&)> onMessage);
2232
};
2333

2434
} // namespace v8_inspector

‎NativeScript/inspector/InspectorServer.mm‎

Lines changed: 157 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,125 @@
33
#include <netinet/in.h>
44
#include <sys/socket.h>
55

6+
#include <atomic>
7+
#include <memory>
8+
69
namespace v8_inspector {
710

11+
namespace {
12+
13+
std::atomic<uint64_t> gLastConnectionId{0};
14+
15+
struct Connection {
16+
uint64_t id;
17+
dispatch_queue_t queue;
18+
dispatch_io_t channel;
19+
std::atomic<bool> open{true};
20+
std::function<void(uint64_t)> onDisconnected;
21+
std::function<void(uint64_t, const std::string&)> onMessage;
22+
};
23+
24+
void CloseConnection(const std::shared_ptr<Connection>& connection) {
25+
if (!connection->open.exchange(false)) {
26+
return;
27+
}
28+
dispatch_io_close(connection->channel, DISPATCH_IO_STOP);
29+
connection->onDisconnected(connection->id);
30+
}
31+
32+
// The functions below take the connection by value: a block captures a C++
33+
// reference as a reference, which would dangle once the caller returns.
34+
35+
// Calls onComplete with exactly `length` bytes, or closes the connection if
36+
// the peer goes away (or errors) first.
37+
void ReadExactly(std::shared_ptr<Connection> connection, size_t length,
38+
void (^onComplete)(dispatch_data_t data)) {
39+
__block dispatch_data_t received = dispatch_data_empty;
40+
dispatch_io_read(connection->channel, 0, length, connection->queue,
41+
^(bool done, dispatch_data_t data, int error) {
42+
if (data != nullptr && dispatch_data_get_size(data) > 0) {
43+
received = dispatch_data_create_concat(received, data);
44+
}
45+
if (!done) {
46+
return;
47+
}
48+
if (error != 0 || dispatch_data_get_size(received) < length) {
49+
if (error != 0 && error != ECANCELED) {
50+
NSLog(@"InspectorServer read error: %s", strerror(error));
51+
}
52+
CloseConnection(connection);
53+
return;
54+
}
55+
onComplete(received);
56+
});
57+
}
58+
59+
// Messages are a big-endian uint32 byte length followed by UTF-16LE text.
60+
void ReadNextMessage(std::shared_ptr<Connection> connection) {
61+
ReadExactly(connection, sizeof(uint32_t), ^(dispatch_data_t header) {
62+
uint32_t length = 0;
63+
[(NSData*)header getBytes:&length length:sizeof(length)];
64+
length = ntohl(length);
65+
if (length == 0) {
66+
CloseConnection(connection);
67+
return;
68+
}
69+
70+
ReadExactly(connection, length, ^(dispatch_data_t body) {
71+
NSString* payload = [[NSString alloc] initWithData:(NSData*)body
72+
encoding:NSUTF16LittleEndianStringEncoding];
73+
const char* utf8 = [payload UTF8String];
74+
if (utf8 != nullptr) {
75+
connection->onMessage(connection->id, utf8);
76+
}
77+
ReadNextMessage(connection);
78+
});
79+
});
80+
}
81+
82+
void Send(std::shared_ptr<Connection> connection, const std::string& message) {
83+
if (!connection->open.load()) {
84+
return;
85+
}
86+
87+
NSString* str = [NSString stringWithUTF8String:message.c_str()];
88+
NSUInteger length = [str lengthOfBytesUsingEncoding:NSUTF16LittleEndianStringEncoding];
89+
90+
uint8_t* buffer = (uint8_t*)malloc(length + sizeof(uint32_t));
91+
92+
*(uint32_t*)buffer = htonl(length);
93+
94+
[str getBytes:&buffer[sizeof(uint32_t)]
95+
maxLength:length
96+
usedLength:NULL
97+
encoding:NSUTF16LittleEndianStringEncoding
98+
options:0
99+
range:NSMakeRange(0, str.length)
100+
remainingRange:NULL];
101+
102+
dispatch_data_t data =
103+
dispatch_data_create(buffer, length + sizeof(uint32_t), connection->queue, ^{
104+
free(buffer);
105+
});
106+
107+
dispatch_io_write(connection->channel, 0, data, connection->queue,
108+
^(bool done, dispatch_data_t data, int error) {
109+
// ECANCELED: the write was still queued when the
110+
// connection closed.
111+
if (error == 0 || error == ECANCELED) {
112+
return;
113+
}
114+
NSLog(@"InspectorServer::Send error: %s", strerror(error));
115+
CloseConnection(connection);
116+
});
117+
}
118+
119+
} // namespace
120+
8121
in_port_t InspectorServer::Init(
9-
std::function<void(std::function<void(const std::string&)>)> onClientConnected,
10-
std::function<void(const std::string&)> onMessage) {
122+
std::function<void(uint64_t connectionId, Sender sender)> onClientConnected,
123+
std::function<void(uint64_t connectionId)> onClientDisconnected,
124+
std::function<void(uint64_t connectionId, const std::string&)> onMessage) {
11125
in_port_t listenPort = 18183;
12126

13127
int serverSocket = -1;
@@ -31,6 +145,10 @@
31145
assert(false);
32146
}
33147

148+
// Only touched by the accept handler, which libdispatch never runs
149+
// concurrently with itself.
150+
__block std::weak_ptr<Connection> activeConnection;
151+
34152
// Set up the dispatch source that will alert us to new incoming connections
35153
dispatch_queue_t q = dispatch_queue_create("server_queue", DISPATCH_QUEUE_CONCURRENT);
36154
dispatch_source_t acceptSource =
@@ -43,68 +161,48 @@
43161
unsigned int clntLen = sizeof(echoClntAddr);
44162

45163
// Wait for a client to connect
46-
if ((clientSocket = accept(serverSocket, (struct sockaddr*)&echoClntAddr, &clntLen)) >= 0) {
47-
dispatch_io_t channel =
48-
dispatch_io_create(DISPATCH_IO_STREAM, clientSocket, q, ^(int error) {
49-
if (error) {
50-
NSLog(@"InspectorServer channel cleanup error: %s", strerror(error));
51-
}
52-
close(clientSocket);
53-
});
54-
55-
onClientConnected([channel, q](std::string message) { Send(channel, q, message); });
56-
57-
__block dispatch_io_handler_t receiver = ^(bool done, dispatch_data_t data, int error) {
58-
if (error) {
59-
NSLog(@"InspectorServer read message header error: %s", strerror(error));
60-
}
61-
62-
const void* bytes = [(NSData*)data bytes];
63-
if (!bytes) {
64-
dispatch_io_close(channel, DISPATCH_IO_STOP);
65-
return;
66-
}
67-
68-
uint32_t length = ntohl(*(uint32_t*)bytes);
69-
70-
// Configure the channel...
71-
dispatch_io_set_low_water(channel, length);
72-
73-
// Setup read handler
74-
dispatch_io_read(channel, 0, length, q, ^(bool done, dispatch_data_t data, int error) {
75-
BOOL close = NO;
76-
if (error) {
77-
NSLog(@"InspectorServer read message body error: %s", strerror(error));
78-
close = YES;
79-
}
80-
81-
const size_t size = data ? dispatch_data_get_size(data) : 0;
82-
if (size) {
83-
NSString* payload = [[NSString alloc] initWithData:(NSData*)data
84-
encoding:NSUTF16LittleEndianStringEncoding];
85-
86-
onMessage([payload UTF8String]);
87-
88-
#pragma clang diagnostic push
89-
#pragma clang diagnostic ignored "-Warc-retain-cycles"
90-
dispatch_io_read(channel, 0, 4, q, receiver);
91-
#pragma clang diagnostic pop
92-
} else {
93-
close = YES;
94-
}
164+
if ((clientSocket = accept(serverSocket, (struct sockaddr*)&echoClntAddr, &clntLen)) < 0) {
165+
NSLog(@"InspectorServer accept() failed");
166+
continue;
167+
}
95168

96-
if (close) {
97-
dispatch_io_close(channel, DISPATCH_IO_STOP);
169+
// A write racing the peer's close must fail with EPIPE, not kill the app.
170+
int noSigPipe = 1;
171+
setsockopt(clientSocket, SOL_SOCKET, SO_NOSIGPIPE, &noSigPipe, sizeof(noSigPipe));
172+
173+
auto connection = std::make_shared<Connection>();
174+
connection->id = ++gLastConnectionId;
175+
// Serial, so the read handlers that assemble one message never overlap.
176+
connection->queue =
177+
dispatch_queue_create("NativeScript.v8.inspector.connection", DISPATCH_QUEUE_SERIAL);
178+
connection->onDisconnected = onClientDisconnected;
179+
connection->onMessage = onMessage;
180+
connection->channel =
181+
dispatch_io_create(DISPATCH_IO_STREAM, clientSocket, connection->queue, ^(int error) {
182+
if (error) {
183+
NSLog(@"InspectorServer channel cleanup error: %s", strerror(error));
98184
}
185+
close(clientSocket);
99186
});
100-
};
101187

102-
if (channel) {
103-
dispatch_io_read(channel, 0, 4, q, receiver);
104-
}
105-
} else {
106-
NSLog(@"InspectorServer accept() failed");
188+
if (connection->channel == nullptr) {
189+
close(clientSocket);
190+
continue;
107191
}
192+
193+
// The accept handler is serial, so back-to-back accepts take over in
194+
// order and the newest client stays active.
195+
std::shared_ptr<Connection> previous = activeConnection.lock();
196+
activeConnection = connection;
197+
if (previous != nullptr) {
198+
CloseConnection(previous);
199+
}
200+
onClientConnected(connection->id,
201+
[connection](const std::string& message) { Send(connection, message); });
202+
203+
dispatch_async(connection->queue, ^{
204+
ReadNextMessage(connection);
205+
});
108206
}
109207
});
110208

@@ -121,32 +219,4 @@
121219
return listenPort;
122220
}
123221

124-
void InspectorServer::Send(dispatch_io_t channel, dispatch_queue_t queue,
125-
const std::string& message) {
126-
NSString* str = [NSString stringWithUTF8String:message.c_str()];
127-
NSUInteger length = [str lengthOfBytesUsingEncoding:NSUTF16LittleEndianStringEncoding];
128-
129-
uint8_t* buffer = (uint8_t*)malloc(length + sizeof(uint32_t));
130-
131-
*(uint32_t*)buffer = htonl(length);
132-
133-
[str getBytes:&buffer[sizeof(uint32_t)]
134-
maxLength:length
135-
usedLength:NULL
136-
encoding:NSUTF16LittleEndianStringEncoding
137-
options:0
138-
range:NSMakeRange(0, str.length)
139-
remainingRange:NULL];
140-
141-
dispatch_data_t data = dispatch_data_create(buffer, length + sizeof(uint32_t), queue, ^{
142-
free(buffer);
143-
});
144-
145-
dispatch_io_write(channel, 0, data, queue, ^(bool done, dispatch_data_t data, int error) {
146-
if (error) {
147-
NSLog(@"InspectorServer::Send error: %s", strerror(error));
148-
}
149-
});
150-
}
151-
152222
} // namespace v8_inspector

0 commit comments

Comments
 (0)