I recently diagnosed a Spring Boot issue that has led me to the conclusion that the default constructor of DefaultResourceLoader is dangerous and needs to be used with more caution than its javadoc currently suggests. The problem is due to the default constructor capturing the thread context class loader (A) and holding a reference to it in a field. This can lead to problems if the DefaultResourceLoader instance is then referenced, directly or indirectly, from a static field of a type that was loaded by a different class loader (B). When this happens, class loader A will now be held in memory for as long as class loader B is reachable. Furthermore, calls to the DefaultResourceLoader may result in class loader A being called when that is no longer expected to happen.
Given the danger described above, ideally new DefaultResourceLoader() would not capture the TCCL. I think it would be preferable if it used DefaultResourceLoader.class.getClassLoader() but I suspect that ship has sailed long ago. Perhaps the default constructor could be deprecated with a view to removing it in the future? That would force code creating a DefaultResourceLoader to think about the ClassLoader that will be used.
Comment From: jhoeller
On review, I'd like to preserve the default constructor but redefine it to use the thread context class loader at the time of actual resource access, as happens with new DefaultResourceLoader(null) already. This should not make a difference to most common use and still allows for simple new DefaultResourceLoader() instantiation, not requiring changes in existing (pre-compiled) code.
That said, since there are straightforward workarounds, I'd rather only do this in 5.3.
Comment From: wilkinsona
redefine it to use the thread context class loader at the time of actual resource access, as happens with
new DefaultResourceLoader(null)
When considering how to fix https://github.com/spring-projects/spring-boot/issues/20900, I rejected switching to new DefaultResourceLoader(null) due to the potential for quite a big change in behaviour. Every DefaultResourceLoader would switch from using the same class loader for its entire existence to potentially using a different class loader each time it's called. In some situations that will make no difference in practice, in others it could cause intermittent resource loading failures depending on the TCCL that's in place. My feeling is that such failures would be hard to diagnose. As such, I'd be a little wary of making such a change in a minor release.
Comment From: jhoeller
The main problem is that default constructor usage is quite common there, not just in DefaultResourceLoader but also in its subclasses (including the application contexts), and not least of it all since there is a setClassLoader method to set the actual loader in a configuration phase. There may also be DefaultResourceLoader instances which are only used for other kinds of resources, never touching a ClassLoader to begin with.
So we need a default constructor in any case, the question is just which default behavior it should have. And the on-demand TCCL access option seems to be a sensible bet since that is very common in other Servlet-related frameworks as well, and since it avoids this potential ClassLoader leak... It's not ideal but I'd be willing to try it in 5.3.