From 9b7663f55bd5378a125653035fddfc443d7c4870 Mon Sep 17 00:00:00 2001 From: Roettges Florian Date: Thu, 30 Jul 2020 14:56:42 +0200 Subject: [PATCH 01/10] Fixed EDT Classloading issues after EDT shutdown. The EventQueue.class stores the classloader in a final field classLoader. The EventQueue itself is stored static inside SunToolkit.class. Even if the active EDT thread classloader is modified to a JNLP classloader, classloading will fail once an AWTAutoShutdown occurs. (No edt activity for 1 second). If any new activity starts, the EDT Thread is recreated and initialized with the classloader stored in field classLoader in EventQueue class. This was not the JNLP classloader anymore. The result are ClassNotFound Exceptions. As there does not seem to be a "nice solution" because of the EventQueue implementation details the current solution temporary sets a DelegatingClassLoader to the thread which will initialize the eventqueue. Once initialized the original classloader is restored to the thread. During webstart loading the DelegatingClassLoader uses the original classloader once the jnlp classloader is active it gets injected into the DelegatingClassLoader. See: https://github.com/karakun/OpenWebStart/issues/201 --- .../java/net/sourceforge/jnlp/Launcher.java | 11 ++- .../jnlp/runtime/AppContextFactory.java | 22 +++++ .../classloader/DelegatingClassLoader.java | 89 +++++++++++++++++++ 3 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java create mode 100644 core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java diff --git a/core/src/main/java/net/sourceforge/jnlp/Launcher.java b/core/src/main/java/net/sourceforge/jnlp/Launcher.java index 9e4a14bab..91fb8bc19 100644 --- a/core/src/main/java/net/sourceforge/jnlp/Launcher.java +++ b/core/src/main/java/net/sourceforge/jnlp/Launcher.java @@ -27,13 +27,14 @@ import net.adoptopenjdk.icedteaweb.resources.UpdatePolicy; import net.adoptopenjdk.icedteaweb.ui.swing.SwingUtils; import net.sourceforge.jnlp.config.DeploymentConfiguration; +import net.sourceforge.jnlp.runtime.AppContextFactory; import net.sourceforge.jnlp.runtime.AppletInstance; import net.sourceforge.jnlp.runtime.ApplicationInstance; -import net.sourceforge.jnlp.runtime.classloader.JNLPClassLoader; import net.sourceforge.jnlp.runtime.JNLPRuntime; +import net.sourceforge.jnlp.runtime.classloader.DelegatingClassLoader; +import net.sourceforge.jnlp.runtime.classloader.JNLPClassLoader; import net.sourceforge.jnlp.services.InstanceExistsException; import net.sourceforge.jnlp.services.ServiceUtil; -import sun.awt.SunToolkit; import javax.swing.text.html.parser.ParserDelegator; import java.applet.Applet; @@ -436,6 +437,9 @@ private void setContextClassLoaderForAllThreads(ThreadGroup tg, ClassLoader clas } } + //inject classloader into edt delegating classloader + DelegatingClassLoader.getInstance().setClassLoader(classLoader); + } /** @@ -643,8 +647,7 @@ public void run() { try { // Do not create new AppContext if we're using NetX and icedteaplugin. // The plugin needs an AppContext too, but it has to be created earlier. - SunToolkit.createNewAppContext(); - + AppContextFactory.createNewAppContext(); doPerApplicationAppContextHacks(); if (file.isApplication()) { diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java b/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java new file mode 100644 index 000000000..94b3f9b09 --- /dev/null +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java @@ -0,0 +1,22 @@ +package net.sourceforge.jnlp.runtime; + +import net.sourceforge.jnlp.runtime.classloader.DelegatingClassLoader; +import sun.awt.SunToolkit; + +public class AppContextFactory +{ + public static void createNewAppContext() + { + //set temporary classloader for EventQueue initialization + //already a call to AppContext.getAppContext(...) initializes the EventQueue.class + ClassLoader originalLoader = Thread.currentThread().getContextClassLoader(); + DelegatingClassLoader delegatingLoader = DelegatingClassLoader.getInstance(); + delegatingLoader.setClassLoader(originalLoader); + + Thread.currentThread().setContextClassLoader(delegatingLoader); + + SunToolkit.createNewAppContext(); + //restore original classloader + Thread.currentThread().setContextClassLoader(originalLoader); + } +} diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java new file mode 100644 index 000000000..130c5e2d7 --- /dev/null +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java @@ -0,0 +1,89 @@ +package net.sourceforge.jnlp.runtime.classloader; + +import java.net.URL; + +public class DelegatingClassLoader extends ClassLoader +{ + public static final DelegatingClassLoader instance = new DelegatingClassLoader(Thread.currentThread().getContextClassLoader()); + private ClassLoader classLoader; + + public static DelegatingClassLoader getInstance() + { + return instance; + } + + DelegatingClassLoader(ClassLoader loader) + { + super(loader); + this.classLoader = loader; + } + + public void setClassLoader(ClassLoader loader) + { + this.classLoader = loader; + } + + protected Class findClass(String name) throws ClassNotFoundException + { + return this.classLoader.loadClass(name); + } + + protected URL findResource(String name) + { + return this.classLoader.getResource(name); + } + + public int hashCode() + { + + int result = 1; + result = 31 * result + (this.classLoader == null ? 0 : this.classLoader.hashCode()); + result = 31 * result + (this.getParent() == null ? 0 : this.getParent().hashCode()); + return result; + } + + public boolean equals(Object obj) + { + if (this == obj) + { + return true; + } + else if (obj == null) + { + return false; + } + else if (this.getClass() != obj.getClass()) + { + return false; + } + else + { + DelegatingClassLoader other = (DelegatingClassLoader) obj; + if (this.classLoader == null) + { + if (other.classLoader != null) + { + return false; + } + } + else if (!this.classLoader.equals(other.classLoader)) + { + return false; + } + + if (this.getParent() == null) + { + if (other.getParent() != null) + { + return false; + } + } + else if (!this.getParent().equals(other.getParent())) + { + return false; + } + + return true; + } + } +} From a16964c30218744a7719c604c072073552b31568 Mon Sep 17 00:00:00 2001 From: Roettges Florian Date: Fri, 31 Jul 2020 09:48:00 +0200 Subject: [PATCH 02/10] Changed equals to use Objects.equals --- .../classloader/DelegatingClassLoader.java | 22 +++---------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java index 130c5e2d7..35efe6fa7 100644 --- a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java @@ -1,6 +1,7 @@ package net.sourceforge.jnlp.runtime.classloader; import java.net.URL; +import java.util.Objects; public class DelegatingClassLoader extends ClassLoader { @@ -35,7 +36,6 @@ protected URL findResource(String name) public int hashCode() { - int result = 1; result = 31 * result + (this.classLoader == null ? 0 : this.classLoader.hashCode()); result = 31 * result + (this.getParent() == null ? 0 : this.getParent().hashCode()); @@ -59,30 +59,14 @@ else if (this.getClass() != obj.getClass()) else { DelegatingClassLoader other = (DelegatingClassLoader) obj; - if (this.classLoader == null) - { - if (other.classLoader != null) - { - return false; - } - } - else if (!this.classLoader.equals(other.classLoader)) + if(Objects.equals(this.classLoader,other.classLoader)==false) { return false; } - - if (this.getParent() == null) - { - if (other.getParent() != null) - { - return false; - } - } - else if (!this.getParent().equals(other.getParent())) + if(Objects.equals(this.getParent(),other.getParent()) == false) { return false; } - return true; } } From 391dfdc52def4d2025f0b0fc7c0d45f951e423ae Mon Sep 17 00:00:00 2001 From: Gr33nbl00d Date: Fri, 31 Jul 2020 10:05:16 +0200 Subject: [PATCH 03/10] Update core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java Co-authored-by: Hendrik Ebbers --- .../jnlp/runtime/classloader/DelegatingClassLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java index 35efe6fa7..f2eee0e8b 100644 --- a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java @@ -59,7 +59,7 @@ else if (this.getClass() != obj.getClass()) else { DelegatingClassLoader other = (DelegatingClassLoader) obj; - if(Objects.equals(this.classLoader,other.classLoader)==false) + if(!Objects.equals(this.classLoader,other.classLoader)) { return false; } From bb52a312cc50c5db36de3a74fbce253bc6620b0f Mon Sep 17 00:00:00 2001 From: Roettges Florian Date: Fri, 31 Jul 2020 10:12:49 +0200 Subject: [PATCH 04/10] Fixed code formatting --- .../jnlp/runtime/AppContextFactory.java | 6 +-- .../classloader/DelegatingClassLoader.java | 42 +++++++------------ 2 files changed, 16 insertions(+), 32 deletions(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java b/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java index 94b3f9b09..305ed8983 100644 --- a/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java @@ -3,10 +3,8 @@ import net.sourceforge.jnlp.runtime.classloader.DelegatingClassLoader; import sun.awt.SunToolkit; -public class AppContextFactory -{ - public static void createNewAppContext() - { +public class AppContextFactory { + public static void createNewAppContext() { //set temporary classloader for EventQueue initialization //already a call to AppContext.getAppContext(...) initializes the EventQueue.class ClassLoader originalLoader = Thread.currentThread().getContextClassLoader(); diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java index 35efe6fa7..c72ac2a41 100644 --- a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java @@ -3,68 +3,54 @@ import java.net.URL; import java.util.Objects; -public class DelegatingClassLoader extends ClassLoader -{ +public class DelegatingClassLoader extends ClassLoader { public static final DelegatingClassLoader instance = new DelegatingClassLoader(Thread.currentThread().getContextClassLoader()); private ClassLoader classLoader; - public static DelegatingClassLoader getInstance() - { + public static DelegatingClassLoader getInstance() { return instance; } - DelegatingClassLoader(ClassLoader loader) - { + DelegatingClassLoader(ClassLoader loader) { super(loader); this.classLoader = loader; } - public void setClassLoader(ClassLoader loader) - { + public void setClassLoader(ClassLoader loader) { this.classLoader = loader; } - protected Class findClass(String name) throws ClassNotFoundException - { + protected Class findClass(String name) throws ClassNotFoundException { return this.classLoader.loadClass(name); } - protected URL findResource(String name) - { + protected URL findResource(String name) { return this.classLoader.getResource(name); } - public int hashCode() - { + public int hashCode() { int result = 1; result = 31 * result + (this.classLoader == null ? 0 : this.classLoader.hashCode()); result = 31 * result + (this.getParent() == null ? 0 : this.getParent().hashCode()); return result; } - public boolean equals(Object obj) - { - if (this == obj) - { + public boolean equals(Object obj) { + if (this == obj) { return true; } - else if (obj == null) - { + else if (obj == null) { return false; } - else if (this.getClass() != obj.getClass()) - { + else if (this.getClass() != obj.getClass()) { return false; } - else - { + else { DelegatingClassLoader other = (DelegatingClassLoader) obj; - if(Objects.equals(this.classLoader,other.classLoader)==false) - { + if(Objects.equals(this.classLoader,other.classLoader)==false) { return false; } - if(Objects.equals(this.getParent(),other.getParent()) == false) - { + if(Objects.equals(this.getParent(),other.getParent()) == false) { return false; } return true; From e816a283100949ef4c98f65356a39d99bd08be37 Mon Sep 17 00:00:00 2001 From: Roettges Florian Date: Fri, 31 Jul 2020 10:17:57 +0200 Subject: [PATCH 05/10] changed second if to use ! instead of == false --- .../jnlp/runtime/classloader/DelegatingClassLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java index 4d6a82919..26ce80395 100644 --- a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java @@ -50,7 +50,7 @@ else if (this.getClass() != obj.getClass()) { if(!Objects.equals(this.classLoader,other.classLoader)) { return false; } - if(Objects.equals(this.getParent(),other.getParent()) == false) { + if(!Objects.equals(this.getParent(),other.getParent())) { return false; } return true; From c50b496f5b650e0b4a37eb1f9d77146fa3d999dd Mon Sep 17 00:00:00 2001 From: Gr33nbl00d Date: Fri, 31 Jul 2020 11:33:29 +0200 Subject: [PATCH 06/10] Update core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java Co-authored-by: Hendrik Ebbers --- .../jnlp/runtime/classloader/DelegatingClassLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java index 26ce80395..c5ae3d26b 100644 --- a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java @@ -11,7 +11,7 @@ public static DelegatingClassLoader getInstance() { return instance; } - DelegatingClassLoader(ClassLoader loader) { + private DelegatingClassLoader(ClassLoader loader) { super(loader); this.classLoader = loader; } From fa662ddf617d322eb13bba97fce918a414874cf8 Mon Sep 17 00:00:00 2001 From: Roettges Florian Date: Fri, 31 Jul 2020 11:36:11 +0200 Subject: [PATCH 07/10] Added asserts for non null checks --- .../jnlp/runtime/classloader/DelegatingClassLoader.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java index 26ce80395..a30b1306c 100644 --- a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java @@ -1,5 +1,7 @@ package net.sourceforge.jnlp.runtime.classloader; +import net.adoptopenjdk.icedteaweb.Assert; + import java.net.URL; import java.util.Objects; @@ -13,10 +15,12 @@ public static DelegatingClassLoader getInstance() { DelegatingClassLoader(ClassLoader loader) { super(loader); + Assert.requireNonNull(loader, "loader"); this.classLoader = loader; } public void setClassLoader(ClassLoader loader) { + Assert.requireNonNull(loader, "loader"); this.classLoader = loader; } From 5330dec000f1f4f8ba515c0b0ea011f273605e3f Mon Sep 17 00:00:00 2001 From: Roettges Florian Date: Fri, 31 Jul 2020 11:43:21 +0200 Subject: [PATCH 08/10] Added finally block to ensure classloader is restored in case of exceptions --- .../jnlp/runtime/AppContextFactory.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java b/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java index 305ed8983..c1d340397 100644 --- a/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java @@ -8,13 +8,15 @@ public static void createNewAppContext() { //set temporary classloader for EventQueue initialization //already a call to AppContext.getAppContext(...) initializes the EventQueue.class ClassLoader originalLoader = Thread.currentThread().getContextClassLoader(); - DelegatingClassLoader delegatingLoader = DelegatingClassLoader.getInstance(); - delegatingLoader.setClassLoader(originalLoader); + try { + DelegatingClassLoader delegatingLoader = DelegatingClassLoader.getInstance(); + delegatingLoader.setClassLoader(originalLoader); - Thread.currentThread().setContextClassLoader(delegatingLoader); - - SunToolkit.createNewAppContext(); - //restore original classloader - Thread.currentThread().setContextClassLoader(originalLoader); + Thread.currentThread().setContextClassLoader(delegatingLoader); + SunToolkit.createNewAppContext(); + }finally { + //restore original classloader + Thread.currentThread().setContextClassLoader(originalLoader); + } } } From f66304b9e8ba3c5e23689e7ae2870881f338c8c1 Mon Sep 17 00:00:00 2001 From: Roettges Florian Date: Fri, 31 Jul 2020 11:46:07 +0200 Subject: [PATCH 09/10] Optimized assert non null check in constructor of DelegatingClassLoader --- .../jnlp/runtime/classloader/DelegatingClassLoader.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java index 9a9b5b781..443f72292 100644 --- a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java @@ -14,8 +14,7 @@ public static DelegatingClassLoader getInstance() { } private DelegatingClassLoader(ClassLoader loader) { - super(loader); - Assert.requireNonNull(loader, "loader"); + super(Assert.requireNonNull(loader, "loader")); this.classLoader = loader; } From f434dd0486b09e0c94b17638e1f62e57f0b47fb0 Mon Sep 17 00:00:00 2001 From: Roettges Florian Date: Fri, 31 Jul 2020 11:55:37 +0200 Subject: [PATCH 10/10] Optimized assert non null check in setClassload() of DelegatingClassLoader --- .../jnlp/runtime/classloader/DelegatingClassLoader.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java index 443f72292..37c82fc20 100644 --- a/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java +++ b/core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java @@ -19,8 +19,7 @@ private DelegatingClassLoader(ClassLoader loader) { } public void setClassLoader(ClassLoader loader) { - Assert.requireNonNull(loader, "loader"); - this.classLoader = loader; + this.classLoader = Assert.requireNonNull(loader, "loader"); } protected Class findClass(String name) throws ClassNotFoundException {