Add the notion of QuarkusClassLoaderType and avoid potentially creating CLs in LogCapturingOutputFilter

[gsmet]

These application.get*ClassLoader() methods are a bit dangerous because they get or create a class loader.

Related to #41233

[gsmet]

@geoand probably another one for you. The commit messages have more details!

[geoand]

Makes sense, but I'd like @aloubyansky to also take a look (especially at the first commit)

[aloubyansky]

I'd be cautious going with enums in this case. I'd probably prefer some int constants and probably even avoid adding isXXX methods. It will look less nice but will avoid exposing this new public API to extensions for now.

[gsmet]

Mmmmh, do we really consider the QuarkusClassLoader as a fully supported API? I mean a lot of things have to do with our internal CL infrastructure and if we had to make changes, well, we would.

I can make the Enum private if it helps.
But the whole idea was to make it easier to classify the class loaders. Ending up with integers won't really help with this.

I can understand your issue with the added methods but I mean I need a way to differentiate the class loaders and one that is readable. I don't want to end with cl.getType() == 2. And if I don't have the methods, I have to expose the Enum or to create constants for the int, which will more or less be API anyway.

Now we could make the method package protected and have an external class that hosts the method and is clearly marked as not being part of the API.

If you have a better proposal, I'm interested but I'd like to have some details as to how we makes things usable.

[aloubyansky]

Is there another place where this could be useful besides LogCapturingOutputFilter?

I consider base runtime, etc an internal impl detail that probably shouldn't be exposed as a type through a public API (or ideally at all). If you can make it package-protected, that'd be better imo. Although just from this PR I don't see enough reasons to do that tbh. Am I missing other use-cases?

[aloubyansky]

application.get*ClassLoader() methods are a bit dangerous because they get or create a class loader

This also could be addressed by adjusting the names of those methods deprecating the current ones. For example, those that always create a new instance could start with create*ClassLoader() instead of get*ClassLoader(), others could be renamed to getOrCreate*ClassLoader().

Are you seeing a case when getBaseRuntimeClassLoader() creates a new instance when it shouldn't?

[gsmet]

@aloubyansky yes I actually did see it happening. It happens when you try to log something from the QuarkusClassLoader constructor.
And this is important to actually be able to debug all these CL issues that we have.

Now, I can probably rename the methods. TBH I wanted to do that but thought that it might break something somewhere.

I will rename the current methods and also expose true getters for this use case.

That being said, I don't see QuarkusClassLoader as API at all. Our internal CL infrastructure shouldn't be seen as an API.
So I don't really understand why you absolutely don't want us adding methods to it. Because IMHO it's nice to have clear classification of the CLs.
But I will focus on fixing the issue at hand.

[aloubyansky]

I don't see QuarkusClassLoader as API at all

I kind of agree with you on this one. Although there are public methods in that class that are used in extension code and I suspect not only in the quarkus repo and for a reason.
If there are good reasons to have those methods, we should consider them of course. From this specific change it just wasn't obvious to me. Once we add something in there is likely a deprecation procedure to refactor it later.