Skip to content

Commit bdb7eb7

Browse files
committed
Pause before initial burst of inspector messages
This change adds a 300ms pause before workerd starts sending out messages when a CDP client attaches. Some clients seem to lose track of the first few messages if they are sent right away. Bug: #1201 Test: manual
1 parent 07c50e3 commit bdb7eb7

File tree

1 file changed

+18
-5
lines changed

1 file changed

+18
-5
lines changed

src/workerd/io/worker.c++

+18-5
Original file line numberDiff line numberDiff line change
@@ -2052,8 +2052,8 @@ private:
20522052

20532053
class Worker::Isolate::InspectorChannelImpl final: public v8_inspector::V8Inspector::Channel {
20542054
public:
2055-
InspectorChannelImpl(kj::Own<const Worker::Isolate> isolateParam, kj::WebSocket& webSocket)
2056-
: ioHandler(webSocket), state(kj::heap<State>(this, kj::mv(isolateParam))) {
2055+
InspectorChannelImpl(kj::Own<const Worker::Isolate> isolateParam, kj::WebSocket& webSocket, kj::Timer& timer)
2056+
: ioHandler(webSocket, timer), state(kj::heap<State>(this, kj::mv(isolateParam))) {
20572057
ioHandler.connect(*this);
20582058
}
20592059

@@ -2286,8 +2286,8 @@ private:
22862286
// the InspectorChannelImpl and the InspectorClient.
22872287
class WebSocketIoHandler final {
22882288
public:
2289-
WebSocketIoHandler(kj::WebSocket& webSocket)
2290-
: webSocket(webSocket) {
2289+
WebSocketIoHandler(kj::WebSocket& webSocket, kj::Timer& timer)
2290+
: webSocket(webSocket), timer(timer) {
22912291
// Assume we are being instantiated on the InspectorService thread, the thread that will do
22922292
// I/O for CDP messages. Messages are delivered to the InspectorChannelImpl on the Isolate thread.
22932293
outgoingQueueNotifier = XThreadNotifier::create();
@@ -2424,6 +2424,18 @@ private:
24242424
}
24252425

24262426
kj::Promise<void> outgoingLoop() {
2427+
// Pause before sending outgoing messages when a connection is first received. V8 starts
2428+
// sending messages as soon as it sees there is an inspector client. The process at the
2429+
// other end of the pipe may not be ready for messages right away (it looks like Chrome does
2430+
// not render CDP messages, and responses, exchanged before the inspector window is rendered).
2431+
//
2432+
// The pause time, 300ms, is experimentally determined on a couple of different dev boxes.
2433+
// Stepping up from 50ms in 50ms increments, 250ms is the lowest value that seems to
2434+
// consistently result in the expected behaviour. 300ms allows for a little more noise.
2435+
//
2436+
// (Bug: https://github.com/cloudflare/workerd/issues/1201, needs some more investigation).
2437+
co_await timer.afterDelay(300 * kj::MILLISECONDS);
2438+
24272439
for (;;) {
24282440
co_await outgoingQueueNotifier->awaitNotification();
24292441
try {
@@ -2454,6 +2466,7 @@ private:
24542466
kj::Own<XThreadNotifier> outgoingQueueNotifier;
24552467

24562468
kj::WebSocket& webSocket; // only accessed on the InspectorService thread.
2469+
kj::Timer& timer;
24572470
std::atomic_bool receivedClose; // accessed on any thread (only transitions false -> true).
24582471
kj::Maybe<InspectorChannelImpl&> channel; // only accessed on the isolate thread.
24592472
};
@@ -2645,7 +2658,7 @@ kj::Promise<void> Worker::Isolate::attachInspector(
26452658

26462659
lockedSelf.impl->inspectorClient.setInspectorTimerInfo(timer, timerOffset);
26472660

2648-
auto channel = kj::heap<Worker::Isolate::InspectorChannelImpl>(kj::atomicAddRef(*this), webSocket);
2661+
auto channel = kj::heap<Worker::Isolate::InspectorChannelImpl>(kj::atomicAddRef(*this), webSocket, timer);
26492662
lockedSelf.currentInspectorSession = *channel;
26502663
lockedSelf.impl->inspectorClient.setChannel(*channel);
26512664

0 commit comments

Comments
 (0)