Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ConcurrentModificationException in updateLoggers() #3235

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

ppkarwasz
Copy link
Contributor

The InternalLoggerRegistry implementation introduced in version 2.24.2 did not return a copy of the registry, when InternalLoggerRegistry.getLoggers() was called. This could lead to a ConcurrentModificationException if a thread creates a new logger, while another thread calls LoggerContext.updateLoggers().

Closes #3234

@garydgregory
Copy link
Member

Hi all,

Does this mean a 2.24.3 is around the corner?

@ppkarwasz
Copy link
Contributor Author

Does this mean a 2.24.3 is around the corner?

@garydgregory, I would wait a couple of weeks to see if other bugs are reported, but, yes, I think we should release a 2.24.3.

We can also release 2.25.0, but that will bring other problems and I have some long term plans for 2.24.x.

@ppkarwasz
Copy link
Contributor Author

@tnleeuw,

I removed the usage of forEach() in 4fa2d54. Could you review the changes?

@tnleeuw
Copy link

tnleeuw commented Dec 6, 2024

@tnleeuw,

I removed the usage of forEach() in 4fa2d54. Could you review the changes?

Looks good to me!

The `InternalLoggerRegistry` implementation introduced in version `2.24.2` did not return a copy of the registry, when `InternalLoggerRegistry.getLoggers()` was called. This could lead to a `ConcurrentModificationException` if a thread creates a new logger, while another thread calls `LoggerContext.updateLoggers()`.

Closes #3234
@ppkarwasz ppkarwasz force-pushed the fix/2.24.x/3234_concurrent-logger-modification branch from 4fa2d54 to bad8b56 Compare December 6, 2024 16:06
@ppkarwasz ppkarwasz merged commit bad8b56 into 2.24.x Dec 6, 2024
3 of 5 checks passed
@ppkarwasz ppkarwasz deleted the fix/2.24.x/3234_concurrent-logger-modification branch December 6, 2024 16:06
@kelunik
Copy link

kelunik commented Dec 9, 2024

@garydgregory, I would wait a couple of weeks to see if other bugs are reported, but, yes, I think we should release a 2.24.3.

Why? Can't you just release 2.24.4 if another bug is found?

@garydgregory
Copy link
Member

@kelunik
Because it takes time and effort for the release manager to create a release candidate and it takes time for PMC members to review that candidate. It's not free even though it might look free from the outside.

@ppkarwasz
Copy link
Contributor Author

@garydgregory, I would wait a couple of weeks to see if other bugs are reported, but, yes, I think we should release a 2.24.3.

Why? Can't you just release 2.24.4 if another bug is found?

An Apache release process requires at least three maintainers and has a 72 hours voting period. Unless the reported problem affects most users, my personal policy is to wait a week or two to give users the time to report other problems.

I will start the release process for 2.24.3 tomorrow, which will contain a fix to this bug and the one reported in #3252 (a strange interaction between Spring Boot, ActiveMQ and Log4j API).

@kelunik
Copy link

kelunik commented Dec 9, 2024

Thanks for providing these details!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants