Yeah, I started a review but then I thought it probably would be better to respond here.
You are on the right track but there is a problem. StatusLogger is a singleton - there is one instance anchored in a static. You are invoking the shutdown logic from the shutdown of the LoggerContext which is not a singleton. Log4j supports multiple LoggerContexts in an application. For example, if you are old school and running multiple web applications in Tomcat and have Log4j on Tomcat’s class path then you will have multiple LoggerContexts with a single StatusLogger. So if one web app gets redeployed then its LoggerContext will be shutdown and a new one created all while another app is continuing to run. If you’ll notice the StatusConfiguration class in log4j-core tries to accommodate for this during startup, but it doesn’t do anything at shutdown. StatusLogger currently isn’t smart enough to handle one app writing to one destination and a different on writing to a different one. Since StatusLogger is a singleton it can’t really know which app a status log event is for. There are a couple of ways I can think of to handle this but none of them is perfect. Modify StatusConfiguration to keep track of what each StatusConfiguration set up and reset to whatever the prior StatusConfiguration had. The problem with this is that applications might shutdown in a different order than they were started, so figuring out what the prior configuration was could be difficult. Add the call to prepareToStop() as a new Callback to Log4j’s shutdown registry. However, this callback would need to run last. The shutdown registry currently doesn’t support a way to specify the order of callbacks. Support for that would need to be added for this to work. Ralph > On Feb 23, 2021, at 10:48 PM, Tim Perry <tim.v...@gmail.com> wrote: > > Ralph, > > I implemented what you suggested. Feel free to suggest improvements. > https://github.com/apache/logging-log4j2/pull/469 > > Tim > > On Tue, Feb 23, 2021 at 2:14 PM Ralph Goers <ralph.go...@dslextreme.com> > wrote: > >> I would suggest that if it is writing to something other than System.out >> that it be redirected back there and then the OutputStream be closed. >> However, I’ve not looked at the code recently so I am not sure what it >> takes to do that. >> >> Ralph >> >>> On Feb 23, 2021, at 2:22 PM, Tim Perry <tim.v...@gmail.com> wrote: >>> >>> Thank you, Volkan. >>> >>> I'm not quite ready to submit a PR. I was hoping some of you with more >>> knowledge of log4j-core would weigh in on what we should do about >> shutting >>> down the StatusLogger. >>> >>> My thought is we choose one of two options: >>> >>> Option A: >>> 1) check if any StatusLogger is writing to standard out or standard >> error. >>> If not, add one. >>> 2) stop any loggers that don't write to standard out or standard error. >>> >>> Option B: >>> 1) stop any loggers that don't write to standard out or standard error. >>> >>> Option A could cause the log messages to be split across two >> destinations, >>> but they all get sent somewhere. Option B could lose shutdown messages >> when >>> writing to a file, but by that point it may not matter. >>> >>> If any of you have a better idea, I'm happy to implement it. If nobody >>> weighs in on the best option, I'll probably submit Option A as a pull >>> request on Friday or Saturday. >>> >>> Tim >>> >> >> >>
