Skip to content

Commit bfbc827

Browse files
entlicherentlicher
committed
[GR-15899] Repair handling of re-connect.
PullRequest: graal/3616
2 parents 52b6eba + 8fa5b7f commit bfbc827

File tree

6 files changed

+126
-26
lines changed

6 files changed

+126
-26
lines changed

‎tools/src/com.oracle.truffle.tools.chromeinspector.test/src/com/oracle/truffle/tools/chromeinspector/test/InspectorMessageTransportTest.java‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.net.URI;
2929
import java.nio.ByteBuffer;
3030
import java.util.ArrayList;
31+
import java.util.Collections;
3132
import java.util.List;
3233
import java.util.regex.Pattern;
3334

@@ -115,6 +116,46 @@ private static void inspectorEndpointTest(String path, RaceControl rc) {
115116
}
116117
}
117118

119+
@Test
120+
public void inspectorReconnectTest() throws IOException, InterruptedException {
121+
Session session = new Session(null);
122+
DebuggerEndpoint endpoint = new DebuggerEndpoint("simplePath", null);
123+
Engine engine = endpoint.onOpen(session);
124+
125+
try (Context context = Context.newBuilder().engine(engine).build()) {
126+
Value result = context.eval("sl", "function main() {\n x = 1;\n return x;\n}");
127+
Assert.assertEquals("Result", "1", result.toString());
128+
129+
MessageEndpoint peerEndpoint = endpoint.peer;
130+
peerEndpoint.sendClose();
131+
Assert.assertNotSame(peerEndpoint, endpoint.peer);
132+
result = context.eval("sl", "function main() {\n x = 2;\n return x;\n}");
133+
Assert.assertEquals("Result", "2", result.toString());
134+
}
135+
// We will not get the last 4 messages as we do not do initial suspension on re-connect
136+
int expectedNumMessages = 2 * MESSAGES.length - 4;
137+
synchronized (session.messages) {
138+
while (session.messages.size() < expectedNumMessages) {
139+
// The reply messages are sent asynchronously. We need to wait for them.
140+
session.messages.wait();
141+
}
142+
}
143+
144+
Assert.assertEquals(session.messages.toString(), expectedNumMessages, session.messages.size());
145+
for (int i = 0; i < MESSAGES.length; i++) {
146+
if (!session.messages.get(i).startsWith(MESSAGES[i])) {
147+
Assert.assertTrue("i = " + i + ", Expected start with '" + MESSAGES[i] + "', got: '" + session.messages.get(i) + "'", false);
148+
}
149+
}
150+
// We will not get the last 4 messages as we do not do initial suspension on re-connect
151+
for (int i = 0; i < MESSAGES.length - 4; i++) {
152+
int j = i + MESSAGES.length;
153+
if (!session.messages.get(j).startsWith(MESSAGES[i])) {
154+
Assert.assertTrue("j = " + j + ", Expected start with '" + MESSAGES[i] + "', got: '" + session.messages.get(j) + "'", false);
155+
}
156+
}
157+
}
158+
118159
@Test
119160
public void inspectorVetoedTest() {
120161
Engine.Builder engineBuilder = Engine.newBuilder().serverTransport(new MessageTransport() {
@@ -137,7 +178,7 @@ public MessageEndpoint open(URI uri, MessageEndpoint peerEndpoint) throws IOExce
137178
private static final class Session {
138179

139180
private final RaceControl rc;
140-
final List<String> messages = new ArrayList<>(MESSAGES.length);
181+
final List<String> messages = Collections.synchronizedList(new ArrayList<>(MESSAGES.length));
141182
private final BasicRemote remote = new BasicRemote(messages);
142183
private boolean opened = true;
143184

@@ -187,7 +228,10 @@ private static final class BasicRemote {
187228

188229
void sendText(String text) throws IOException {
189230
if (!text.startsWith("{\"method\":\"Debugger.scriptParsed\"")) {
190-
messages.add("toClient(" + text + ")");
231+
synchronized (messages) {
232+
messages.add("toClient(" + text + ")");
233+
messages.notifyAll();
234+
}
191235
}
192236
if (text.startsWith("{\"method\":\"Debugger.paused\"")) {
193237
handler.onMessage("{\"id\":100,\"method\":\"Debugger.resume\"}");
@@ -200,6 +244,7 @@ private static final class DebuggerEndpoint {
200244

201245
private final String path;
202246
private final RaceControl rc;
247+
MessageEndpoint peer;
203248

204249
DebuggerEndpoint(String path, RaceControl rc) {
205250
this.path = path;
@@ -212,6 +257,7 @@ public Engine onOpen(final Session session) {
212257
@Override
213258
public MessageEndpoint open(URI requestURI, MessageEndpoint peerEndpoint) throws IOException, MessageTransport.VetoException {
214259
Assert.assertEquals("Invalid protocol", "ws", requestURI.getScheme());
260+
DebuggerEndpoint.this.peer = peerEndpoint;
215261
String uriStr = requestURI.toString();
216262
if (path == null) {
217263
Assert.assertTrue(uriStr, URI_PATTERN.matcher(uriStr).matches());

‎tools/src/com.oracle.truffle.tools.chromeinspector/src/com/oracle/truffle/tools/chromeinspector/instrument/InspectorInstrument.java‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,7 @@ protected void onCreate(Env env) {
209209
HostAndPort hostAndPort = options.get(Inspect);
210210
connectionWatcher = new ConnectionWatcher();
211211
try {
212-
InetSocketAddress socketAddress = hostAndPort.createSocket();
213-
server = new Server(env, "Main Context", socketAddress, options.get(Attach), options.get(Suspend), options.get(WaitAttached), options.get(HideErrors), options.get(Internal),
212+
server = new Server(env, "Main Context", hostAndPort, options.get(Attach), options.get(Suspend), options.get(WaitAttached), options.get(HideErrors), options.get(Internal),
214213
options.get(Initialization), options.get(Path), options.hasBeenSet(Secure), options.get(Secure), new KeyStoreOptions(options), options.get(SourcePath),
215214
connectionWatcher);
216215
} catch (IOException e) {
@@ -235,8 +234,7 @@ public synchronized InspectorServerConnection open(int port, String host, boolea
235234
connectionWatcher = new ConnectionWatcher();
236235
hostAndPort = new HostAndPort(host, port);
237236
try {
238-
InetSocketAddress socketAddress = hostAndPort.createSocket();
239-
server = new Server(env, "Main Context", socketAddress, false, false, wait, options.get(HideErrors), options.get(Internal),
237+
server = new Server(env, "Main Context", hostAndPort, false, false, wait, options.get(HideErrors), options.get(Internal),
240238
options.get(Initialization), null, options.hasBeenSet(Secure), options.get(Secure), new KeyStoreOptions(options), options.get(SourcePath), connectionWatcher);
241239
} catch (IOException e) {
242240
PrintWriter info = new PrintWriter(env.err());
@@ -378,27 +376,28 @@ private static final class Server {
378376
private final String wsURL;
379377
private final InspectorExecutionContext executionContext;
380378

381-
Server(final Env env, final String contextName, final InetSocketAddress socketAdress, final boolean attach, final boolean debugBreak, final boolean waitAttached, final boolean hideErrors,
379+
Server(final Env env, final String contextName, final HostAndPort hostAndPort, final boolean attach, final boolean debugBreak, final boolean waitAttached, final boolean hideErrors,
382380
final boolean inspectInternal, final boolean inspectInitialization, final String pathOrNull, final boolean secureHasBeenSet, final boolean secureValue,
383381
final KeyStoreOptions keyStoreOptions, final List<URI> sourcePath, final ConnectionWatcher connectionWatcher) throws IOException {
382+
InetSocketAddress socketAddress = hostAndPort.createSocket();
384383
PrintWriter info = new PrintWriter(env.err(), true);
385384
if (pathOrNull == null || pathOrNull.isEmpty()) {
386385
wsspath = "/" + Long.toHexString(System.identityHashCode(env)) + "-" + Long.toHexString(System.nanoTime() ^ System.identityHashCode(env));
387386
} else {
388387
String head = pathOrNull.startsWith("/") ? "" : "/";
389388
wsspath = head + pathOrNull;
390389
}
391-
boolean secure = (!secureHasBeenSet && socketAdress.getAddress().isLoopbackAddress()) ? false : secureValue;
390+
boolean secure = (!secureHasBeenSet && socketAddress.getAddress().isLoopbackAddress()) ? false : secureValue;
392391

393392
PrintWriter err = (hideErrors) ? null : info;
394393
executionContext = new InspectorExecutionContext(contextName, inspectInternal, inspectInitialization, env, sourcePath, err);
395394
if (attach) {
396-
wss = new InspectWSClient(socketAdress, wsspath, executionContext, debugBreak, secure, keyStoreOptions, connectionWatcher, info);
395+
wss = new InspectWSClient(socketAddress, wsspath, executionContext, debugBreak, secure, keyStoreOptions, connectionWatcher, info);
397396
wsURL = ((InspectWSClient) wss).getURI().toString();
398397
} else {
399398
URI wsuri;
400399
try {
401-
wsuri = new URI(secure ? "wss" : "ws", null, socketAdress.getAddress().getHostAddress(), socketAdress.getPort(), wsspath, null, null);
400+
wsuri = new URI(secure ? "wss" : "ws", null, socketAddress.getAddress().getHostAddress(), socketAddress.getPort(), wsspath, null, null);
402401
} catch (URISyntaxException ex) {
403402
throw new IOException(ex);
404403
}
@@ -412,15 +411,16 @@ private static final class Server {
412411
}
413412
if (serverEndpoint == null) {
414413
interceptor.close(wsspath);
415-
wss = WebSocketServer.get(socketAdress, wsspath, executionContext, debugBreak, secure, keyStoreOptions, connectionWatcher, iss);
416-
String wsStr = buildAddress(socketAdress.getAddress().getHostAddress(), wss.getPort(), wsspath, secure);
414+
wss = WebSocketServer.get(socketAddress, wsspath, executionContext, debugBreak, secure, keyStoreOptions, connectionWatcher, iss);
415+
String wsStr = buildAddress(socketAddress.getAddress().getHostAddress(), wss.getPort(), wsspath, secure);
417416
String address = DEV_TOOLS_PREFIX + wsStr;
418417
wsURL = wsStr.replace("=", "://");
419418
info.println("Debugger listening on port " + wss.getPort() + ".");
420419
info.println("To start debugging, open the following URL in Chrome:");
421420
info.println(" " + address);
422421
info.flush();
423422
} else {
423+
restartServerEndpointOnClose(hostAndPort, env, wsuri, executionContext, connectionWatcher, iss, interceptor);
424424
interceptor.opened(serverEndpoint);
425425
wss = interceptor;
426426
wsURL = wsuri.toString();
@@ -489,6 +489,25 @@ private static String buildAddress(String hostAddress, int port, String path, bo
489489
return prefix + hostAddress + ":" + port + path;
490490
}
491491

492+
private static void restartServerEndpointOnClose(HostAndPort hostAndPort, Env env, URI wsuri, InspectorExecutionContext executionContext, ConnectionWatcher connectionWatcher,
493+
InspectServerSession iss, WSInterceptorServer interceptor) {
494+
iss.onClose(() -> {
495+
// debugBreak = false, do not break on re-connect
496+
InspectServerSession newSession = InspectServerSession.create(executionContext, false, connectionWatcher);
497+
interceptor.newSession(newSession);
498+
MessageEndpoint serverEndpoint;
499+
try {
500+
serverEndpoint = env.startServer(wsuri, newSession);
501+
} catch (MessageTransport.VetoException vex) {
502+
return;
503+
} catch (IOException ioex) {
504+
throw new InspectorIOException(hostAndPort.getHostPort(), ioex);
505+
}
506+
interceptor.opened(serverEndpoint);
507+
restartServerEndpointOnClose(hostAndPort, env, wsuri, executionContext, connectionWatcher, newSession, interceptor);
508+
});
509+
}
510+
492511
public void close() throws IOException {
493512
if (wss != null) {
494513
wss.close(wsspath);

‎tools/src/com.oracle.truffle.tools.chromeinspector/src/com/oracle/truffle/tools/chromeinspector/server/InspectServerSession.java‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ public final class InspectServerSession implements MessageEndpoint {
6262
final InspectorExecutionContext context;
6363
private volatile MessageEndpoint messageEndpoint;
6464
private volatile JSONMessageListener jsonMessageListener;
65-
private CommandProcessThread processThread;
65+
private volatile CommandProcessThread processThread;
66+
private Runnable onClose;
6667

6768
private InspectServerSession(RuntimeDomain runtime, DebuggerDomain debugger, ProfilerDomain profiler,
6869
InspectorExecutionContext context) {
@@ -79,23 +80,34 @@ public static InspectServerSession create(InspectorExecutionContext context, boo
7980
return new InspectServerSession(runtime, debugger, profiler, context);
8081
}
8182

83+
public void onClose(Runnable onCloseTask) {
84+
this.onClose = onCloseTask;
85+
}
86+
8287
@Override
8388
public void sendClose() {
84-
runtime.disable();
85-
debugger.disable();
86-
profiler.disable();
87-
context.reset();
88-
messageEndpoint = null;
89-
processThread.dispose();
90-
processThread = null;
89+
Runnable onCloseRunnable = null;
90+
synchronized (this) {
91+
runtime.disable();
92+
debugger.disable();
93+
profiler.disable();
94+
context.reset();
95+
messageEndpoint = null;
96+
processThread.dispose();
97+
processThread = null;
98+
onCloseRunnable = onClose;
99+
}
100+
if (onCloseRunnable != null) {
101+
onCloseRunnable.run();
102+
}
91103
}
92104

93105
// For tests only
94106
public DebuggerDomain getDebugger() {
95107
return debugger;
96108
}
97109

98-
public void setMessageListener(MessageEndpoint messageListener) {
110+
public synchronized void setMessageListener(MessageEndpoint messageListener) {
99111
this.messageEndpoint = messageListener;
100112
if (messageListener != null && processThread == null) {
101113
EventHandler eh = new EventHandlerImpl();
@@ -107,7 +119,7 @@ public void setMessageListener(MessageEndpoint messageListener) {
107119
}
108120
}
109121

110-
public void setJSONMessageListener(JSONMessageListener messageListener) {
122+
public synchronized void setJSONMessageListener(JSONMessageListener messageListener) {
111123
this.jsonMessageListener = messageListener;
112124
if (messageListener != null && processThread == null) {
113125
EventHandler eh = new EventHandlerImpl();
@@ -131,14 +143,20 @@ public void sendText(String message) {
131143
}
132144
return;
133145
}
134-
processThread.push(cmd);
146+
CommandProcessThread pt = processThread;
147+
if (pt != null) {
148+
pt.push(cmd);
149+
}
135150
}
136151

137152
public void sendCommand(Command cmd) {
138153
if (context.isSynchronous()) {
139154
sendCommandSync(cmd);
140155
} else {
141-
processThread.push(cmd);
156+
CommandProcessThread pt = processThread;
157+
if (pt != null) {
158+
pt.push(cmd);
159+
}
142160
}
143161
}
144162

‎tools/src/com.oracle.truffle.tools.chromeinspector/src/com/oracle/truffle/tools/chromeinspector/server/WSInterceptorServer.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -40,7 +40,7 @@ public final class WSInterceptorServer implements InspectorWSConnection, Message
4040

4141
private final URI uri;
4242
private final ConnectionWatcher connectionWatcher;
43-
private final InspectServerSession iss;
43+
private InspectServerSession iss;
4444
private MessageEndpoint inspectEndpoint;
4545

4646
public WSInterceptorServer(URI uri, InspectServerSession iss, ConnectionWatcher connectionWatcher) {
@@ -50,6 +50,12 @@ public WSInterceptorServer(URI uri, InspectServerSession iss, ConnectionWatcher
5050
iss.setMessageListener(this);
5151
}
5252

53+
public void newSession(InspectServerSession newIss) {
54+
this.iss.setMessageListener(null);
55+
this.iss = newIss;
56+
this.iss.setMessageListener(this);
57+
}
58+
5359
public void opened(MessageEndpoint endpoint) {
5460
this.inspectEndpoint = endpoint;
5561
iss.setMessageListener(this);

‎tools/src/com.oracle.truffle.tools.chromeinspector/src/com/oracle/truffle/tools/chromeinspector/server/WebSocketServer.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.io.UnsupportedEncodingException;
3737
import java.net.InetSocketAddress;
3838
import java.net.Socket;
39+
import java.net.SocketException;
3940
import java.nio.ByteBuffer;
4041
import java.security.KeyStore;
4142
import java.security.KeyStoreException;
@@ -373,6 +374,15 @@ public void onClose(NanoWSD.WebSocketFrame.CloseCode code, String reason, boolea
373374
iss.sendClose();
374375
}
375376

377+
@Override
378+
public void close(WebSocketFrame.CloseCode code, String reason, boolean initiatedByRemote) throws IOException {
379+
try {
380+
super.close(code, reason, initiatedByRemote);
381+
} catch (SocketException ex) {
382+
// The socket is broken already.
383+
}
384+
}
385+
376386
@Override
377387
public void onMessage(NanoWSD.WebSocketFrame frame) {
378388
String message = frame.getTextPayload();

‎truffle/src/com.oracle.truffle.api.debug/src/com/oracle/truffle/api/debug/Debugger.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ void disposedSession(DebuggerSession session) {
167167
for (Breakpoint b : breakpoints) {
168168
b.sessionClosed(session);
169169
}
170+
alwaysHaltBreakpoint.sessionClosed(session);
170171
}
171172
}
172173

0 commit comments

Comments
 (0)