Skip to content

Commit

Permalink
[GR-15899] Repair handling of re-connect.
Browse files Browse the repository at this point in the history 
PullRequest: graal/3616
  • Loading branch information
@entlicher
entlicher committed May 22, 2019
2 parents 52b6eba + 8fa5b7f commit bfbc827
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 26 deletions.
50 changes: 48 additions & 2 deletions ...test/src/com/oracle/truffle/tools/chromeinspector/test/InspectorMessageTransportTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.net.URI;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.regex.Pattern;

Expand Down Expand Up @@ -115,6 +116,46 @@ private static void inspectorEndpointTest(String path, RaceControl rc) {
}
}

@Test
public void inspectorReconnectTest() throws IOException, InterruptedException {
Session session = new Session(null);
DebuggerEndpoint endpoint = new DebuggerEndpoint("simplePath", null);
Engine engine = endpoint.onOpen(session);

try (Context context = Context.newBuilder().engine(engine).build()) {
Value result = context.eval("sl", "function main() {\n x = 1;\n return x;\n}");
Assert.assertEquals("Result", "1", result.toString());

MessageEndpoint peerEndpoint = endpoint.peer;
peerEndpoint.sendClose();
Assert.assertNotSame(peerEndpoint, endpoint.peer);
result = context.eval("sl", "function main() {\n x = 2;\n return x;\n}");
Assert.assertEquals("Result", "2", result.toString());
}
// We will not get the last 4 messages as we do not do initial suspension on re-connect
int expectedNumMessages = 2 * MESSAGES.length - 4;
synchronized (session.messages) {
while (session.messages.size() < expectedNumMessages) {
// The reply messages are sent asynchronously. We need to wait for them.
session.messages.wait();
}
}

Assert.assertEquals(session.messages.toString(), expectedNumMessages, session.messages.size());
for (int i = 0; i < MESSAGES.length; i++) {
if (!session.messages.get(i).startsWith(MESSAGES[i])) {
Assert.assertTrue("i = " + i + ", Expected start with '" + MESSAGES[i] + "', got: '" + session.messages.get(i) + "'", false);
}
}
// We will not get the last 4 messages as we do not do initial suspension on re-connect
for (int i = 0; i < MESSAGES.length - 4; i++) {
int j = i + MESSAGES.length;
if (!session.messages.get(j).startsWith(MESSAGES[i])) {
Assert.assertTrue("j = " + j + ", Expected start with '" + MESSAGES[i] + "', got: '" + session.messages.get(j) + "'", false);
}
}
}

@Test
public void inspectorVetoedTest() {
Engine.Builder engineBuilder = Engine.newBuilder().serverTransport(new MessageTransport() {
Expand All @@ -137,7 +178,7 @@ public MessageEndpoint open(URI uri, MessageEndpoint peerEndpoint) throws IOExce
private static final class Session {

private final RaceControl rc;
final List<String> messages = new ArrayList<>(MESSAGES.length);
final List<String> messages = Collections.synchronizedList(new ArrayList<>(MESSAGES.length));
private final BasicRemote remote = new BasicRemote(messages);
private boolean opened = true;

Expand Down Expand Up @@ -187,7 +228,10 @@ private static final class BasicRemote {

void sendText(String text) throws IOException {
if (!text.startsWith("{\"method\":\"Debugger.scriptParsed\"")) {
messages.add("toClient(" + text + ")");
synchronized (messages) {
messages.add("toClient(" + text + ")");
messages.notifyAll();
}
}
if (text.startsWith("{\"method\":\"Debugger.paused\"")) {
handler.onMessage("{\"id\":100,\"method\":\"Debugger.resume\"}");
Expand All @@ -200,6 +244,7 @@ private static final class DebuggerEndpoint {

private final String path;
private final RaceControl rc;
MessageEndpoint peer;

DebuggerEndpoint(String path, RaceControl rc) {
this.path = path;
Expand All @@ -212,6 +257,7 @@ public Engine onOpen(final Session session) {
@Override
public MessageEndpoint open(URI requestURI, MessageEndpoint peerEndpoint) throws IOException, MessageTransport.VetoException {
Assert.assertEquals("Invalid protocol", "ws", requestURI.getScheme());
DebuggerEndpoint.this.peer = peerEndpoint;
String uriStr = requestURI.toString();
if (path == null) {
Assert.assertTrue(uriStr, URI_PATTERN.matcher(uriStr).matches());
Expand Down
39 changes: 29 additions & 10 deletions ...nspector/src/com/oracle/truffle/tools/chromeinspector/instrument/InspectorInstrument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,7 @@ protected void onCreate(Env env) {
HostAndPort hostAndPort = options.get(Inspect);
connectionWatcher = new ConnectionWatcher();
try {
InetSocketAddress socketAddress = hostAndPort.createSocket();
server = new Server(env, "Main Context", socketAddress, options.get(Attach), options.get(Suspend), options.get(WaitAttached), options.get(HideErrors), options.get(Internal),
server = new Server(env, "Main Context", hostAndPort, options.get(Attach), options.get(Suspend), options.get(WaitAttached), options.get(HideErrors), options.get(Internal),
options.get(Initialization), options.get(Path), options.hasBeenSet(Secure), options.get(Secure), new KeyStoreOptions(options), options.get(SourcePath),
connectionWatcher);
} catch (IOException e) {
Expand All @@ -235,8 +234,7 @@ public synchronized InspectorServerConnection open(int port, String host, boolea
connectionWatcher = new ConnectionWatcher();
hostAndPort = new HostAndPort(host, port);
try {
InetSocketAddress socketAddress = hostAndPort.createSocket();
server = new Server(env, "Main Context", socketAddress, false, false, wait, options.get(HideErrors), options.get(Internal),
server = new Server(env, "Main Context", hostAndPort, false, false, wait, options.get(HideErrors), options.get(Internal),
options.get(Initialization), null, options.hasBeenSet(Secure), options.get(Secure), new KeyStoreOptions(options), options.get(SourcePath), connectionWatcher);
} catch (IOException e) {
PrintWriter info = new PrintWriter(env.err());
Expand Down Expand Up @@ -378,27 +376,28 @@ private static final class Server {
private final String wsURL;
private final InspectorExecutionContext executionContext;

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

PrintWriter err = (hideErrors) ? null : info;
executionContext = new InspectorExecutionContext(contextName, inspectInternal, inspectInitialization, env, sourcePath, err);
if (attach) {
wss = new InspectWSClient(socketAdress, wsspath, executionContext, debugBreak, secure, keyStoreOptions, connectionWatcher, info);
wss = new InspectWSClient(socketAddress, wsspath, executionContext, debugBreak, secure, keyStoreOptions, connectionWatcher, info);
wsURL = ((InspectWSClient) wss).getURI().toString();
} else {
URI wsuri;
try {
wsuri = new URI(secure ? "wss" : "ws", null, socketAdress.getAddress().getHostAddress(), socketAdress.getPort(), wsspath, null, null);
wsuri = new URI(secure ? "wss" : "ws", null, socketAddress.getAddress().getHostAddress(), socketAddress.getPort(), wsspath, null, null);
} catch (URISyntaxException ex) {
throw new IOException(ex);
}
Expand All @@ -412,15 +411,16 @@ private static final class Server {
}
if (serverEndpoint == null) {
interceptor.close(wsspath);
wss = WebSocketServer.get(socketAdress, wsspath, executionContext, debugBreak, secure, keyStoreOptions, connectionWatcher, iss);
String wsStr = buildAddress(socketAdress.getAddress().getHostAddress(), wss.getPort(), wsspath, secure);
wss = WebSocketServer.get(socketAddress, wsspath, executionContext, debugBreak, secure, keyStoreOptions, connectionWatcher, iss);
String wsStr = buildAddress(socketAddress.getAddress().getHostAddress(), wss.getPort(), wsspath, secure);
String address = DEV_TOOLS_PREFIX + wsStr;
wsURL = wsStr.replace("=", "://");
info.println("Debugger listening on port " + wss.getPort() + ".");
info.println("To start debugging, open the following URL in Chrome:");
info.println(" " + address);
info.flush();
} else {
restartServerEndpointOnClose(hostAndPort, env, wsuri, executionContext, connectionWatcher, iss, interceptor);
interceptor.opened(serverEndpoint);
wss = interceptor;
wsURL = wsuri.toString();
Expand Down Expand Up @@ -489,6 +489,25 @@ private static String buildAddress(String hostAddress, int port, String path, bo
return prefix + hostAddress + ":" + port + path;
}

private static void restartServerEndpointOnClose(HostAndPort hostAndPort, Env env, URI wsuri, InspectorExecutionContext executionContext, ConnectionWatcher connectionWatcher,
InspectServerSession iss, WSInterceptorServer interceptor) {
iss.onClose(() -> {
// debugBreak = false, do not break on re-connect
InspectServerSession newSession = InspectServerSession.create(executionContext, false, connectionWatcher);
interceptor.newSession(newSession);
MessageEndpoint serverEndpoint;
try {
serverEndpoint = env.startServer(wsuri, newSession);
} catch (MessageTransport.VetoException vex) {
return;
} catch (IOException ioex) {
throw new InspectorIOException(hostAndPort.getHostPort(), ioex);
}
interceptor.opened(serverEndpoint);
restartServerEndpointOnClose(hostAndPort, env, wsuri, executionContext, connectionWatcher, newSession, interceptor);
});
}

public void close() throws IOException {
if (wss != null) {
wss.close(wsspath);
Expand Down
42 changes: 30 additions & 12 deletions ...meinspector/src/com/oracle/truffle/tools/chromeinspector/server/InspectServerSession.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ public final class InspectServerSession implements MessageEndpoint {
final InspectorExecutionContext context;
private volatile MessageEndpoint messageEndpoint;
private volatile JSONMessageListener jsonMessageListener;
private CommandProcessThread processThread;
private volatile CommandProcessThread processThread;
private Runnable onClose;

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

public void onClose(Runnable onCloseTask) {
this.onClose = onCloseTask;
}

@Override
public void sendClose() {
runtime.disable();
debugger.disable();
profiler.disable();
context.reset();
messageEndpoint = null;
processThread.dispose();
processThread = null;
Runnable onCloseRunnable = null;
synchronized (this) {
runtime.disable();
debugger.disable();
profiler.disable();
context.reset();
messageEndpoint = null;
processThread.dispose();
processThread = null;
onCloseRunnable = onClose;
}
if (onCloseRunnable != null) {
onCloseRunnable.run();
}
}

// For tests only
public DebuggerDomain getDebugger() {
return debugger;
}

public void setMessageListener(MessageEndpoint messageListener) {
public synchronized void setMessageListener(MessageEndpoint messageListener) {
this.messageEndpoint = messageListener;
if (messageListener != null && processThread == null) {
EventHandler eh = new EventHandlerImpl();
Expand All @@ -107,7 +119,7 @@ public void setMessageListener(MessageEndpoint messageListener) {
}
}

public void setJSONMessageListener(JSONMessageListener messageListener) {
public synchronized void setJSONMessageListener(JSONMessageListener messageListener) {
this.jsonMessageListener = messageListener;
if (messageListener != null && processThread == null) {
EventHandler eh = new EventHandlerImpl();
Expand All @@ -131,14 +143,20 @@ public void sendText(String message) {
}
return;
}
processThread.push(cmd);
CommandProcessThread pt = processThread;
if (pt != null) {
pt.push(cmd);
}
}

public void sendCommand(Command cmd) {
if (context.isSynchronous()) {
sendCommandSync(cmd);
} else {
processThread.push(cmd);
CommandProcessThread pt = processThread;
if (pt != null) {
pt.push(cmd);
}
}
}

Expand Down
10 changes: 8 additions & 2 deletions ...omeinspector/src/com/oracle/truffle/tools/chromeinspector/server/WSInterceptorServer.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -40,7 +40,7 @@ public final class WSInterceptorServer implements InspectorWSConnection, Message

private final URI uri;
private final ConnectionWatcher connectionWatcher;
private final InspectServerSession iss;
private InspectServerSession iss;
private MessageEndpoint inspectEndpoint;

public WSInterceptorServer(URI uri, InspectServerSession iss, ConnectionWatcher connectionWatcher) {
Expand All @@ -50,6 +50,12 @@ public WSInterceptorServer(URI uri, InspectServerSession iss, ConnectionWatcher
iss.setMessageListener(this);
}

public void newSession(InspectServerSession newIss) {
this.iss.setMessageListener(null);
this.iss = newIss;
this.iss.setMessageListener(this);
}

public void opened(MessageEndpoint endpoint) {
this.inspectEndpoint = endpoint;
iss.setMessageListener(this);
Expand Down
10 changes: 10 additions & 0 deletions ....chromeinspector/src/com/oracle/truffle/tools/chromeinspector/server/WebSocketServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.io.UnsupportedEncodingException;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.net.SocketException;
import java.nio.ByteBuffer;
import java.security.KeyStore;
import java.security.KeyStoreException;
Expand Down Expand Up @@ -373,6 +374,15 @@ public void onClose(NanoWSD.WebSocketFrame.CloseCode code, String reason, boolea
iss.sendClose();
}

@Override
public void close(WebSocketFrame.CloseCode code, String reason, boolean initiatedByRemote) throws IOException {
try {
super.close(code, reason, initiatedByRemote);
} catch (SocketException ex) {
// The socket is broken already.
}
}

@Override
public void onMessage(NanoWSD.WebSocketFrame frame) {
String message = frame.getTextPayload();
Expand Down
1 change: 1 addition & 0 deletions truffle/src/com.oracle.truffle.api.debug/src/com/oracle/truffle/api/debug/Debugger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ void disposedSession(DebuggerSession session) {
for (Breakpoint b : breakpoints) {
b.sessionClosed(session);
}
alwaysHaltBreakpoint.sessionClosed(session);
}
}

Expand Down

0 comments on commit bfbc827

Please sign in to comment.