This is exactly why we love getting new committers on board!
When I wrote this code my thinking was that I have no idea what ClassLoader
scheme the app is running in. Heck, JBoss had at least 3 of them in the time
I worked with it years ago. So the idea here is to simply find implementations
wherever they can be found and use the ones that work. I know it isn’t pretty
but I couldn’t figure out a better way. This is one of the major benefits of
JPMS
is that it definitely cleans this up.
I have no idea what is going on with item 4.
Ralph
> On Mar 18, 2022, at 2:50 AM, Piotr P. Karwasz <piotr.karw...@gmail.com> wrote:
>
> Hello,
>
> I am trying to understand the way the context classloader and
> `ServiceLoader` are used in Log4j 2.x[1].
>
> The `log4j-api` uses `LoaderUtil.getClassLoaders` in several places to
> lookup services from the context classloader and the `log4j-api`
> classloader.
>
> This procedure has IMHO some problems:
> 1. The services retrieved this way are assigned to static fields
> (except the call in `WatchEventService`): this causes a memory leak if
> `log4j-api` is in the server's classloader and the retrieved service
> in the web application classloader,
>
> 2. There is the small problem that if any service found by
> `ServiceLoader.load(Class, ClassLoader)` fails the `instanceof` check,
> all the others are thrown away. I solved it in `ServiceLoaderUtil`,
>
> 3. Probably this is not a problem, but I fail to understand why we
> look up services separately for a classloader and each of its
> ancestors.
> If the classloader implements the "delegate to parent" convention,
> we'll end up several instances of the same class.
> If the classloader is a webapp classloader and the service
> interface is in the webapp classloader, all services in the
> classloader ancestors will fail the 'instanceof' test.
> The only situation where this makes sense is if the service
> interface is in the server classloader (e.g. `javax.sql.DataSource`),
> then we can have two different services with the same class name. Is
> this the reason?
>
> 4. The following condition in `ThreadContextDataInjector` is pure magic for
> me:
> if (providers.stream().noneMatch(p ->
> p.getClass().isAssignableFrom(provider.getClass()))) {
> providers.add(provider);
> }
> As I understand it, if we have a base class, we don't want its
> derived classes?
>
> I have don't some refactoring of the ServiceLoader code in:
> https://github.com/apache/logging-log4j2/pull/804
>
>
> [1] https://issues.apache.org/jira/browse/LOG4J2-3427
