Devin rnd3 + Ryan rnd1

deephaven · Nov 10, 2023 · cdc5232 · cdc5232
1 parent 7606163
commit cdc5232
Show file tree

Hide file tree

Showing 8 changed files with 36 additions and 68 deletions.
diff --git a/engine/context/src/main/java/io/deephaven/engine/context/QueryCompiler.java b/engine/context/src/main/java/io/deephaven/engine/context/QueryCompiler.java
@@ -683,62 +683,41 @@ private void maybeCreateClass(String className, String code, String packageName,
         }
     }
 
-    private static volatile JavaCompiler JAVA_COMPILER = null;
-
-    private static JavaCompiler getJavaCompiler() {
-        JavaCompiler localCompiler;
-        if ((localCompiler = JAVA_COMPILER) == null) {
-            synchronized (QueryCompiler.class) {
-                if ((localCompiler = JAVA_COMPILER) == null) {
-                    JAVA_COMPILER = localCompiler = ToolProvider.getSystemJavaCompiler();
-                }
-            }
-        }
-        if (localCompiler == null) {
-            throw new RuntimeException("No Java compiler provided - are you using a JRE instead of a JDK?");
-        }
-        return localCompiler;
-    }
-
-    /**
-     * While the JavaFileManager should be closed to clean up resources, using a singleton avoids repeated processing of
-     * the classpath, which is <b>very</b> expensive.
-     */
-    private static volatile JavaFileManager JAVA_FILE_MANAGER = null;
-
-    private static JavaFileManager getJavaFileManager() {
-        JavaFileManager localManager;
-        if ((localManager = JAVA_FILE_MANAGER) == null) {
-            synchronized (QueryCompiler.class) {
-                if ((localManager = JAVA_FILE_MANAGER) == null) {
-                    JAVA_FILE_MANAGER = localManager = getJavaCompiler().getStandardFileManager(null, null, null);
-                }
-            }
-        }
-        return localManager;
-    }
-
     private void maybeCreateClassHelper(String fqClassName, String finalCode, String[] splitPackageName,
             String rootPathAsString, String tempDirAsString) {
         final StringWriter compilerOutput = new StringWriter();
 
-        final JavaCompiler compiler = getJavaCompiler();
+        final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
+        if (compiler == null) {
+            throw new RuntimeException("No Java compiler provided - are you using a JRE instead of a JDK?");
+        }
 
         final String classPathAsString = getClassPath() + File.pathSeparator + getJavaClassPath();
         final List<String> compilerOptions = Arrays.asList("-d", tempDirAsString, "-cp", classPathAsString);
 
-        final JavaFileManager fileManager = getJavaFileManager();
+        final StandardJavaFileManager fileManager = compiler.getStandardFileManager(null, null, null);
 
-        final boolean result;
-        // the java file manager is not thread safe
-        synchronized (QueryCompiler.class) {
+        boolean result = false;
+        boolean exceptionThrown = false;
+        try {
             result = compiler.getTask(compilerOutput,
                     fileManager,
                     null,
                     compilerOptions,
                     null,
                     Collections.singletonList(new JavaSourceFromString(fqClassName, finalCode)))
                     .call();
+        } catch (final Exception ignored) {
+            exceptionThrown = true;
+        } finally {
+            try {
+                fileManager.close();
+            } catch (final IOException ioe) {
+                if (!exceptionThrown) {
+                    // noinspection ThrowFromFinallyBlock
+                    throw new UncheckedIOException("Could not close JavaFileManager", ioe);
+                }
+            }
         }
         if (!result) {
             throw new RuntimeException("Error compiling class " + fqClassName + ":\n" + compilerOutput);
@@ -771,7 +750,7 @@ private void maybeCreateClassHelper(String fqClassName, String finalCode, String
      * @return a Pair of success, and the compiler output
      */
     private Pair<Boolean, String> tryCompile(File basePath, Collection<File> javaFiles) throws IOException {
-        final JavaCompiler compiler = getJavaCompiler();
+        final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
         if (compiler == null) {
             throw new RuntimeException("No Java compiler provided - are you using a JRE instead of a JDK?");
         }

diff --git a/java-client/session-dagger/src/test/java/io/deephaven/client/DeephavenSessionTestBase.java b/java-client/session-dagger/src/test/java/io/deephaven/client/DeephavenSessionTestBase.java
@@ -3,8 +3,11 @@
  */
 package io.deephaven.client;
 
+import io.deephaven.base.verify.Require;
 import io.deephaven.client.impl.Session;
+import io.deephaven.client.impl.SessionImpl;
 import io.deephaven.server.runner.DeephavenApiServerTestBase;
+import io.deephaven.server.session.SessionState;
 import io.grpc.ManagedChannel;
 import org.junit.After;
 import org.junit.Before;
@@ -17,6 +20,7 @@ public abstract class DeephavenSessionTestBase extends DeephavenApiServerTestBas
 
     private ScheduledExecutorService sessionScheduler;
     protected Session session;
+    protected SessionState serverSessionState;
 
     @Override
     @Before
@@ -25,8 +29,12 @@ public void setUp() throws Exception {
         ManagedChannel channel = channelBuilder().build();
         register(channel);
         sessionScheduler = Executors.newScheduledThreadPool(2);
-        session = DaggerDeephavenSessionRoot.create().factoryBuilder().managedChannel(channel)
-                .scheduler(sessionScheduler).build().newSession();
+        final SessionImpl clientSessionImpl =
+                DaggerDeephavenSessionRoot.create().factoryBuilder().managedChannel(channel)
+                        .scheduler(sessionScheduler).build().newSession();
+        session = clientSessionImpl;
+        serverSessionState = Require.neqNull(server().sessionService().getSessionForToken(
+                clientSessionImpl._hackBearerHandler().getCurrentToken()), "SessionState");
     }
 
     @Override

diff --git a/java-client/session-dagger/src/test/java/io/deephaven/client/impl/TableServiceAsyncTest.java b/java-client/session-dagger/src/test/java/io/deephaven/client/impl/TableServiceAsyncTest.java
@@ -88,7 +88,7 @@ private static TableHandle get(TableHandleFuture future)
     private void checkSucceeded(TableHandle x, int chainLength) {
         assertThat(x.isSuccessful()).isTrue();
         try (final SafeCloseable ignored = getExecutionContext().open()) {
-            final Table result = getSession(session.getCurrentToken()).<Table>getExport(x.exportId().id()).get();
+            final Table result = serverSessionState.<Table>getExport(x.exportId().id()).get();
             ExecutionContext.getContext().getQueryScope().putParam("ChainLength", chainLength);
             final Table expected = TableTools.emptyTable(CHAIN_ROWS).update("Current = ii - 1 + ChainLength");
             TstUtils.assertTableEquals(expected, result);

diff --git a/java-client/session/src/main/java/io/deephaven/client/impl/BearerHandler.java b/java-client/session/src/main/java/io/deephaven/client/impl/BearerHandler.java
@@ -59,7 +59,7 @@ public void setBearerToken(String bearerToken) {
     }
 
     @VisibleForTesting
-    UUID getCurrentToken() {
+    public UUID getCurrentToken() {
         return UUID.fromString(bearerToken);
     }
 

diff --git a/java-client/session/src/main/java/io/deephaven/client/impl/Session.java b/java-client/session/src/main/java/io/deephaven/client/impl/Session.java
@@ -3,10 +3,8 @@
  */
 package io.deephaven.client.impl;
 
-import com.google.common.annotations.VisibleForTesting;
 import io.deephaven.proto.DeephavenChannel;
 
-import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 
 /**
@@ -24,12 +22,6 @@ public interface Session
     @Override
     void close();
 
-    /**
-     * Returns the current auth token.
-     */
-    @VisibleForTesting
-    UUID getCurrentToken();
-
     /**
      * Closes the session.
      *

diff --git a/java-client/session/src/main/java/io/deephaven/client/impl/SessionImpl.java b/java-client/session/src/main/java/io/deephaven/client/impl/SessionImpl.java
@@ -3,6 +3,7 @@
  */
 package io.deephaven.client.impl;
 
+import com.google.common.annotations.VisibleForTesting;
 import io.deephaven.client.impl.script.Changes;
 import io.deephaven.proto.DeephavenChannel;
 import io.deephaven.proto.backplane.grpc.AddTableRequest;
@@ -124,20 +125,15 @@ private SessionImpl(SessionImplConfig config, DeephavenChannel bearerChannel, Du
                 pingFrequency.toNanos(), pingFrequency.toNanos(), TimeUnit.NANOSECONDS);
     }
 
-    // exposed for Flight
-    BearerHandler _hackBearerHandler() {
+    // exposed for Flight and testing
+    public BearerHandler _hackBearerHandler() {
         return bearerHandler;
     }
 
     private ExportStates newExportStates() {
         return new ExportStates(this, bearerChannel.session(), bearerChannel.table(), exportTicketCreator);
     }
 
-    @Override
-    public UUID getCurrentToken() {
-        return bearerHandler.getCurrentToken();
-    }
-
     @Override
     public TableService newStatefulTableService() {
         return new TableServiceImpl(newExportStates());

diff --git a/server/src/main/java/io/deephaven/server/runner/DeephavenApiServer.java b/server/src/main/java/io/deephaven/server/runner/DeephavenApiServer.java
@@ -98,7 +98,7 @@ public GrpcServer server() {
     }
 
     @VisibleForTesting
-    SessionService sessionService() {
+    public SessionService sessionService() {
         return sessionService;
     }
 

diff --git a/server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java b/server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java
@@ -24,13 +24,11 @@
 import io.deephaven.server.plugin.js.JsPluginNoopConsumerModule;
 import io.deephaven.server.runner.scheduler.SchedulerDelegatingImplModule;
 import io.deephaven.server.session.ObfuscatingErrorTransformerModule;
-import io.deephaven.server.session.SessionState;
 import io.deephaven.server.util.Scheduler;
 import io.deephaven.util.SafeCloseable;
 import io.grpc.ManagedChannel;
 import io.grpc.ManagedChannelBuilder;
 import io.grpc.testing.GrpcCleanupRule;
-import org.jetbrains.annotations.NotNull;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -42,7 +40,6 @@
 import java.io.PrintStream;
 import java.time.Duration;
 import java.util.Optional;
-import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -176,10 +173,6 @@ public ScriptSession getScriptSession() {
         return scriptSessionProvider.get();
     }
 
-    public SessionState getSession(@NotNull final UUID token) {
-        return server.sessionService().getSessionForToken(token);
-    }
-
     public ExecutionContext getExecutionContext() {
         return executionContext;
     }