Skip to content

Jakarta Activation erroneously assumes that classes can be loaded from Thread#getContextClassLoader#145

Merged
lukasj merged 6 commits into
jakartaee:masterfrom
jmehrens:issue_144
Feb 14, 2024
Merged

Jakarta Activation erroneously assumes that classes can be loaded from Thread#getContextClassLoader#145
lukasj merged 6 commits into
jakartaee:masterfrom
jmehrens:issue_144

Conversation

@jmehrens

@jmehrens jmehrens commented Jan 8, 2024

Copy link
Copy Markdown
Contributor

#144

Signed-off-by: jmehrens jason_mehrens@hotmail.com

…m Thread#getContextClassLoader

Signed-off-by: jmehrens <jason_mehrens@hotmail.com>
@jmehrens jmehrens self-assigned this Jan 9, 2024
Signed-off-by: jmehrens <jason_mehrens@hotmail.com>
…m Thread#getContextClassLoader jakartaee#144

Signed-off-by: jmehrens <jason_mehrens@hotmail.com>

@lukasj lukasj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update year in copyright headers of modified files, please (ie 2021 -> 2021, 2024)

…m Thread#getContextClassLoader jakartaee#145

Signed-off-by: jmehrens <jason_mehrens@hotmail.com>
…m Thread#getContextClassLoader jakartaee#145

Signed-off-by: jmehrens <jason_mehrens@hotmail.com>
@jmehrens jmehrens requested a review from lukasj February 14, 2024 05:22
…m Thread#getContextClassLoader jakartaee#145

Signed-off-by: jmehrens <jason_mehrens@hotmail.com>
@lukasj lukasj merged commit 4b8285b into jakartaee:master Feb 14, 2024
lukasj pushed a commit to lukasj/jaf-api that referenced this pull request Feb 15, 2024
…m Thread#getContextClassLoader (jakartaee#145)

Signed-off-by: jmehrens <jason_mehrens@hotmail.com>
(cherry picked from commit 4b8285b)
lukasj pushed a commit that referenced this pull request Feb 15, 2024
…m Thread#getContextClassLoader (#145)

Signed-off-by: jmehrens <jason_mehrens@hotmail.com>
(cherry picked from commit 4b8285b)
@jmehrens

Copy link
Copy Markdown
Contributor Author

@basil I ported the JakartaMail classloading changes to JakartaActivation. Your existing jenkins activation workaround should continue function as normally. However, the changes should allow you to migrate away from the using the workaround if you so desire.

If you get some free cycles test and let me know. As always, if you see an issue let us know.

@basil

basil commented Mar 6, 2024

Copy link
Copy Markdown

If you get some free cycles test and let me know. As always, if you see an issue let us know.

@jmehrens Thank you very much for working on this. With the release of Jakarta Mail 2.1.3, I have finally been able to upgrade the Jenkins ecosystem to Jakarta Mail 2.1.3, Jakarta Activation 2.1.3, and Eclipse Angus. The change has been released to production users since Monday with no reported issues. Thank you very much!

I ported the Jakarta Mail classloading changes to Jakarta Activation. Your existing jenkins activation workaround should continue function as normally. However, the changes should allow you to migrate away from the using the workaround if you so desire.

I tried removing my workaround, and although your changes worked correctly, I still needed the workaround for an unexpected reason. You might notice my workaround is in the Jenkins Jakarta Mail plugin, which can see classes from the Jenkins Jakarta Activation plugin (but not vice versa). When I removed the workaround, your new code correctly kicked in and used the calling class loader from Jenkins Jakarta Activation plugin, but since that class loader could not see the Jenkins Jakarta Mail plugin, the mailcap was empty.

Since we don't allow circular dependencies between Jenkins plugins, the only way for me to fix this would be to combine the Jenkins Jakarta Activation plugin and Jenkins Jakarta Mail plugin into a single plugin. I might do that eventually, in which case I am confident your change would allow me to remove the workaround. But for now, this is a huge step forward, so I am going to declare success in the short to medium term and save that problem for another day.

@jmehrens

jmehrens commented Mar 7, 2024

Copy link
Copy Markdown
Contributor Author

@basil That is good news! Thanks for testing and providing feedback.

When I removed the workaround, your new code correctly kicked in and used the calling class loader from Jenkins Jakarta Activation plugin, but since that class loader could not see the Jenkins Jakarta Mail plugin, the mailcap was empty.

Interesting. I'll do some more thinking on this. Presumably, resource lookup would work if Jakarta Mail classloader was given to Jakarta Activation. Next release it would be possible to use stackwalker to locate calling classloader. I'll do some thinking on this.

@basil

basil commented Mar 7, 2024

Copy link
Copy Markdown

Presumably, resource lookup would work if Jakarta Mail classloader was given to Jakarta Activation.

Indeed, and that's effectively what my workaround is doing by using a classloader from Jenkins Jakarta Mail plugin (rather than Jenkins Jakarta Activation plugin), albeit by setting the thread's context class loader. That's also why I'm sure this problem would also be fixed if I combined the two Jenkins plugins into a single Jenkins plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Jakarta Activation erroneously assumes that classes can be loaded from Thread#getContextClassLoader

3 participants