The reason a DefaultConfiguration is created and NOT the identified configuration is that a) we would have to block while the configuration is created and b) creating a Configuration isn’t necessarily very fast depending on what is in it (some of which we cannot control). We do want logging to be available quickly and only logging errors to the console during the initialization period seems like a fair compromise. It is not at all inconceivable that an application will have started multiple threads that perform logging while logging is initializing and in general, we want Log4j to be as lock free as possible.
So your “fairly innocuous” change is not something I would support as I explicitly rejected that idea when I implemented it in the first place. I am also not in favor of a configuration option that makes the start up logic even more complicated and fragile. Ralph > On Dec 28, 2022, at 6:23 PM, Matt Sicker <m...@musigma.org> wrote: > > And with this detail about the possibility for a Logger to be returned while > LoggerContext::start is being executed is indeed a reason for why > DefaultConfiguration _normally_ exists. I’m looking at two additional > scenarios: > > 1. What if you provide a configuration location String or URI to the > LoggerContext constructor? We already have a configuration source in this > case (or at least we do if the URI corresponds to a local file), but we > ignore this source until we start the LoggerContext. It is this situation > where I thought it may be possible to start loading ConfigurationFactory > plugins here to load and parse the specified configuration as the initial > Configuration instance rather than an instance of DefaultConfiguration. This > wasn’t previously possible due to various code dependencies, but given the > fact that we have access to an initialized Injector instance inside the > LoggerContext constructor (this may be a key detail I haven’t been too > explicit about) means that we can start loading whatever subset of plugins we > want at this point. > > 2. If you don’t provide a configuration location, we could still attempt to > load the ConfigurationFactory plugins to bootstrap here. This is even more > realistic now that we’ve removed the plugin package scanning feature, so we > know that plugin service classes will already be available to load at this > time. > > What I’m thinking here is a fairly innocuous-looking change may allow us to > load the configuration earlier. As it is now, the LoggerContext is > getOrCreated by the ContextSelector before Log4jContextSelector checks its > state to see if it should be started (in which case it will then load > ConfigurationFactory plugins to start the real configuration). What I think > might be possible is moving this ConfigurationFactory bit deeper into the > call chain by creating it when the LoggerContext is being created. This would > mean the LoggerContext would have the custom configuration instance _before_ > LoggerContext::start is invoked. > > One of the main tradeoffs here would be startup time I suppose. By deferring > configuration loading until LoggerContext::start is invoked, we do indeed get > out of the way as soon as possible to start accepting log messages and > returning control to user code. So I suppose even if it turns out to be > possible to load the configuration right away, this would by definition cause > blocking as the configuration is loaded, so this would probably make sense as > a configurable option. > — > Matt Sicker > >> On Dec 28, 2022, at 11:44, Ralph Goers <ralph.go...@dslextreme.com> wrote: >> >> >> >>> On Dec 28, 2022, at 10:37 AM, Ralph Goers <ralph.go...@dslextreme.com> >>> wrote: >>> >>> >>> >>> Ralph >>> >>>> On Dec 28, 2022, at 7:01 AM, Piotr P. Karwasz <piotr.karw...@gmail.com> >>>> wrote: >>>> >>>> Hi Matt, >>>> >>>> On Sun, 18 Dec 2022 at 20:30, Matt Sicker <m...@musigma.org> wrote: >>>>> During this bootstrapping, if the configuration location is available >>>>> (such as for a unit test), should LoggerContext set up the configuration >>>>> provided? Or is there some sort of cyclic dependency here preventing us >>>>> from loading ConfigurationFactory right away? So far, the only cyclic >>>>> dependencies I’ve found are related to plugins created in the >>>>> DefaultConfiguration (or the NullConfiguration in some cases), but those >>>>> are already commented as such (like in PatternLayout). >>>> >>>> I think we should rely more on our `LifeCycle` abstraction: >>>> `Configuration` starts in the "initializing" state and does not have >>>> any subcomponents (especially those that require a `LoggerContext` to >>>> be present) until `initialize()` is called. >>> >>> Actually, a LoggerContext is hard-wired with a DefaultConfiguration. So as >>> soon as it exists it has a Configuration. Once the Configuration identified >>> from the configLocation is created the DefaultConfiguration is replaced. >>> This is the discussion Matt was having in his previous email and didn’t >>> understand why it is required to work that way. Note that once the >>> LoggerContext is constructed it will be “wired” by the ContextSelector and >>> will be usable by any thread. >>> >>>> >>>> We can do the same thing with `LoggerContext`: after the constructor >>>> exits it does not have a configuration and is in the "initializing" >>>> state. >>> >>> After the Constructor exists is has a DefaultConfiguration. >>> >>>> The configuration will be created on an `initialize()` call >>>> (maybe chained from `start()`). By then we can safely pass a reference >>>> to `LoggerContext` to the `AbstractConfiguration` constructor. >>>> >>>> Logging does not occur until `start()` is called. >>> >>> I don’t believe this is true. Once the ContextSelector registers it any >>> calls to log will use it. >>> >>> Ralph >> >> Note that LoggerContext.start() uses tryLock. If initialization is in >> progress then start() will simply return. So other threads will log while >> initialization is occurring. This is intentional. >> >> Ralph > >
