Skips a redundant call to LoggerContext#reset().

  • Prior to Logback 1.5.7, calling #stop() on any LoggerContext implicitly called #reset(). On the other hand, since Logback 1.5.7, only calling #stop() on a started LoggerContext implicitly calls #reset(). Going forward, given the next version of Spring Boot will not be binary or source compatible with Logback 1.5.6 and earlier, it's only necessary to call #reset() on a stopped LoggerContext.

Comment From: philwebb

Going forward, given the next version of Spring Boot will not be binary or source compatible with Logback 1.5.6 and earlier

Is this the case? Is Logback 1.5.7 not binary compatible with Logback 1.5.6?

Comment From: mches

Thanks for the review 🙇‍♂️

Comment From: mches

Going forward, given the next version of Spring Boot will not be binary or source compatible with Logback 1.5.6 and earlier

Is this the case? Is Logback 1.5.7 not binary compatible with Logback 1.5.6?

Yes, because the return type of LoggerContext#getConfigurationLock() changed from Object to ReentrantLock. It's source compatible, but not binary compatible.

Spring Boot 3.3.2 with Logback 1.5.7 blows up with NoSuchMethodError merely loading the application context.

Comment From: snicoll

So this is in addition of the actual upgrade to Logback then?

Comment From: mches

So this is in addition of the actual upgrade to Logback then?

Yes, I thought this made sense to do after reviewing dd2ed5f.

Comment From: mches

Thank you for the review. 🙇🏻 All feedback has been implemented. ✨🧹

Comment From: wilkinsona

I think we should leave this as-is. Arguably, calling both stop() and then reset() has been unnecessary for some time as in 1.5.6 and earlier versions a call to stop() will also call reset() but it hasn't caused any problems.

In 1.5.7, stop() will only call reset() if the context is started. A context is started in its constructor so, if it isn't started, it must have already been stopped and, therefore, must have already been reset. I think that means that we could, perhaps, do nothing at all if the context isn't started. It's hard to justify making a change as there doesn't appear to be any benefit yet there is some risk in making the change. I'm going to close this one, for now at least. We can reconsider in the future and perhaps do nothing at all if the context isn't started. Thanks anyway, @mches.