Fixed EDT Classloading issues after EDT shutdown. #679
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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: karakun/OpenWebStart#201
|
Implementation of DelegatingClassLoader has same overriden methods than the original implementation of DelegationClassloader: |
|
As i am not so deeply into classloading, i am asking myself it this implementation is correct from security perspective. Feel free to modify |
|
@Gr33nbl00d great to see this PR. Thank you very much. I will try to have a look at this next week. |
sclassen
reviewed
Jul 30, 2020
core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java
Outdated
Show resolved
Hide resolved
core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java
Outdated
Show resolved
Hide resolved
core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java
Outdated
Show resolved
Hide resolved
core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java
Outdated
Show resolved
Hide resolved
…legatingClassLoader.java Co-authored-by: Hendrik Ebbers <[email protected]>
core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java
Outdated
Show resolved
Hide resolved
core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java
Outdated
Show resolved
Hide resolved
core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java
Outdated
Show resolved
Hide resolved
…legatingClassLoader.java Co-authored-by: Hendrik Ebbers <[email protected]>
core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java
Show resolved
Hide resolved
core/src/main/java/net/sourceforge/jnlp/runtime/AppContextFactory.java
Outdated
Show resolved
Hide resolved
core/src/main/java/net/sourceforge/jnlp/runtime/classloader/DelegatingClassLoader.java
Outdated
Show resolved
Hide resolved
10 tasks
|
@FlorianRoettges & @Gr33nbl00d thanks for taking care of all the mentioned points in the review. I like the current state. In general I think that the given "Workaround" (let's call it lime that) is a good way for ITW / OWS to handle the general problem. I will have a deeper look and additional tests with @sclassen before we merge this PR. Hope to have time for this next week. This would make it possible to have it in the August release of ITW & OWS. |
|
@sclassen merge? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
karakun/OpenWebStart#201