jakartaee / jaf-api

Jakarta Activation Specification project
https://jakartaee.github.io/jaf-api/
BSD 3-Clause "New" or "Revised" License
31 stars 33 forks source link

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

Closed jmehrens closed 7 months ago

jmehrens commented 8 months ago

https://github.com/jakartaee/jaf-api/issues/144

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

jmehrens commented 7 months ago

@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 commented 6 months ago

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 commented 6 months ago

@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 commented 6 months ago

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.