It sounds like we could use some cleanup around here. Number 3 is
particularly interesting.

As for number 4, that seems to be a form of uniqueness checking (no
two providers of the same class).

On Fri, Mar 18, 2022 at 4: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

Reply via email to