Skip redundant LoggerContext reset

[mches]

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.

[philwebb]

Thanks for the PR. I've added a couple of issues to review.

[philwebb]

Is this change accidental? It doesn't seem related to the other changes?

[mches]

This appeared in #41885, which was reverted.

Without this, I ran into this issue in my environment, which failed the tests that use Mockito.mock within this class for being unable to delete mockitoboot*.jar from the JUnit temp directory.

With the change, the test is less finicky on Windows.

That said, I can take it out if required.

[wilkinsona]

I don't think it's needed. Regardless, it isn't related to this change so I think it should be taken out. #41905 has hopefully helped here.

[philwebb]

There doesn't seem to be any changes in this file so the copyright header should not be updated.

[mches]

This file was updated in #41885, which was reverted, but updated again without updating the years. So I feel it's accurate, but I'm happy to take it out if required.

[wilkinsona]

I should have bumped the year as part of dd2ed5f. That should now be done separately as it isn't related to this change so please take it out of this PR.

[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?

[mches]

Thanks for the review 🙇‍♂️

[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.

[snicoll]

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

[wilkinsona]

I don't think it's needed. Regardless, it isn't related to this change so I think it should be taken out. #41905 has hopefully helped here.

[wilkinsona]

I should have bumped the year as part of dd2ed5f. That should now be done separately as it isn't related to this change so please take it out of this PR.

[mches]

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

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

[mches]

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

[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.