Skip to content

Conversation

@andrus
Copy link
Contributor

@andrus andrus commented Jun 14, 2025

@stariy95 , I was able to address a part of the issue - avoiding failures in the declaration snippet. However an attempt to instantiate one of the declared classes in the following cell still throws. So we need to dig a bit deeper.

new Animal();
java.lang.RuntimeException: java.lang.NoClassDefFoundError, REPL[/](http://localhost:8888/)$JShell$13$Animal
	at org.dflib.jjava.execution.CodeEvaluator.evalSingle(CodeEvaluator.java:144)
	at org.dflib.jjava.execution.CodeEvaluator.eval(CodeEvaluator.java:176)
	at org.dflib.jjava.JavaKernel.evalRaw(JavaKernel.java:337)
	at org.dflib.jjava.JavaKernel.eval(JavaKernel.java:342)
	at org.dflib.jjava.jupyter.kernel.BaseKernel.handleExecuteRequest(BaseKernel.java:383)
	at org.dflib.jjava.jupyter.channels.ShellChannel.lambda$bind$0(ShellChannel.java:64)
	at org.dflib.jjava.jupyter.channels.Loop.lambda$new$0(Loop.java:21)
	at org.dflib.jjava.jupyter.channels.Loop.run(Loop.java:78)
Caused by: jdk.jshell.EvalException: REPL[/](http://localhost:8888/)$JShell$13$Animal
	at .(#31:1)

@andrus andrus added this to the 1.0-a6 milestone Jun 14, 2025
@andrus andrus modified the milestones: 1.0-a6, 1.0-a7 Nov 8, 2025
@m-dzianishchyts
Copy link
Contributor

I've got it fixed on my side by not throwing an exception in org.dflib.jjava.kernel.execution.JJavaLoaderDelegate#load. BTW it can be replaced with some warning message to mimic jshell behavior

    @Override
-   public void load(ExecutionControl.ClassBytecodes[] cbcs) throws ExecutionControl.ClassInstallException {
+   public void load(ExecutionControl.ClassBytecodes[] cbcs) {
        boolean[] installed = new boolean[cbcs.length];
        int i = 0;
        for (ExecutionControl.ClassBytecodes cbc : cbcs) {
            declaredClasses.put(cbc.name(), cbc.bytecodes());
            try {
                Class<?> loaderClass = classLoader.findClass(cbc.name());
                loadedClasses.put(cbc.name(), loaderClass);
            } catch (ClassNotFoundException e) {
-               throw new ExecutionControl.ClassInstallException("Unable to load class " + cbc.name()
-                       + ": " + e.getMessage(), installed);
+               logger.warn("Class {} not found, expected to be declared later", cbc.name());
            }
            installed[i++] = true;
        }
    }

@andrus
Copy link
Contributor Author

andrus commented Nov 10, 2025

I think we are moving in the right direction. However, this fix may be too optimistic. There can be irrecoverable class loading failures too, and we should throw on those. Look inside DefaultLoaderDelegate. It actually throws ClassInstallException on all errors, but sets the exception installed flag to distinguish between the two cases. I wonder if that's the secret sauce here.

Notice how JShell has this very precise messaging about the two classes involved:

created class Animal, however, it cannot be referenced until class ShepherdDog is declared

@m-dzianishchyts
Copy link
Contributor

I see. We need to mark all the classes as declared first to make it possible to find them later via findClass(String). This patch should fix the issue.

Index: jjava-kernel/src/main/java/org/dflib/jjava/kernel/execution/JJavaLoaderDelegate.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/jjava-kernel/src/main/java/org/dflib/jjava/kernel/execution/JJavaLoaderDelegate.java b/jjava-kernel/src/main/java/org/dflib/jjava/kernel/execution/JJavaLoaderDelegate.java
--- a/jjava-kernel/src/main/java/org/dflib/jjava/kernel/execution/JJavaLoaderDelegate.java	(revision f04acb8d6738b2264a6af35ae0490fe803e87a85)
+++ b/jjava-kernel/src/main/java/org/dflib/jjava/kernel/execution/JJavaLoaderDelegate.java	(date 1763670856855)
@@ -30,9 +30,13 @@
     @Override
     public void load(ExecutionControl.ClassBytecodes[] cbcs) throws ExecutionControl.ClassInstallException {
         boolean[] installed = new boolean[cbcs.length];
+
+        for (ExecutionControl.ClassBytecodes cbc : cbcs) {
+            declaredClasses.put(cbc.name(), cbc.bytecodes());
+        }
+
         int i = 0;
         for (ExecutionControl.ClassBytecodes cbc : cbcs) {
-            declaredClasses.put(cbc.name(), cbc.bytecodes());
             try {
                 Class<?> loaderClass = classLoader.findClass(cbc.name());
                 loadedClasses.put(cbc.name(), loaderClass);

@andrus andrus force-pushed the 65 branch 2 times, most recently from be0a12d to 98c2a90 Compare November 22, 2025 15:22
@andrus
Copy link
Contributor Author

andrus commented Nov 22, 2025

@m-dzianishchyts , your latest solution works the same way as JShell. I incorporated it in my PR. Thanks!

@andrus andrus merged commit f028741 into main Nov 22, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants