You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by "Piotr P. Karwasz" <pi...@mailing.copernik.eu> on 2024/11/16 09:33:57 UTC
Log4j release 2.24.1
Hi all,
Due to bug #3143[1], which allows a `Logger` to be garbage-collected
before it is returned by `LoggerContext#getLogger()`, I am planning a
2.24.2 release with just two changes:
* A fix for the above mentioned bug (see PR #3209[2]).
* A fix for a bug related to `ThreadContextMap` (see PR #3210[3]).
Could you check those PRs to make sure we don't miss anything this time?
I am planning the release for Sunday evening or Monday morning, but I'll
wait until those PRs have reviews.
Piotr
[1] https://github.com/apache/logging-log4j2/issues/3143
[2] https://github.com/apache/logging-log4j2/pull/3209
[3] https://github.com/apache/logging-log4j2/pull/3210
Re: Log4j release 2.24.1
Posted by Gary Gregory <ga...@gmail.com>.
Hi Piotr,
Thanks for the details. The good new is that it was caught fairly quickly.
Looking forward to reviewing the RC...
Gary
On Sat, Nov 16, 2024, 12:06 PM Piotr P. Karwasz <pi...@mailing.copernik.eu>
wrote:
> Hi Gary,
>
> On 16.11.2024 17:41, Gary Gregory wrote:
> > [1]https://github.com/apache/logging-log4j2/issues/3143
> >
> > This issue points to the merged PR
> > https://github.com/apache/logging-log4j2/pull/2936, a large change set
> with
> > zero tests. This isn't great since there are no tests to catch
> regressions
> > :-(
>
> When PR #2936 was reviewed, bug #3143 wasn't known.
>
> While PR #2936 (for the `2.25.x` release) is not affected by the bug,
> the bug itself was caused by a backport of PR #2936 to the `2.24.x`
> branch (see PR #2961[1]). The bug itself is due to the reachability
> rules for Java local variables: a variable becomes unreachable **not**
> at the end of the syntactic scope, but right after its last
> **non-trivial** usage (see [2] for more details). That is not an easy
> problem to catch in a review IMHO and certainly not something you can
> easily test.
>
> [1] https://github.com/apache/logging-log4j2/pull/2961
>
> [2] https://shipilev.net/jvm/anatomy-quarks/8-local-var-reachability/
>
> > [2]https://github.com/apache/logging-log4j2/pull/3209
> >
> > LGTM: If I apply the patch, the test is green, then when I revert the
> > 'main' changes, the test fails. Nice work :-)
> >
> > [3]https://github.com/apache/logging-log4j2/pull/3210
> >
> > LGTM: If I apply the patch, the 2 tests are green, then when I revert the
> > 'main' changes, the 2 tests fail. Nice work :-)
> >
> > So it all looks good :-)
>
> Could you formally approve those PRs? Thanks.
>
> Piotr
>
Re: Log4j release 2.24.1
Posted by "Piotr P. Karwasz" <pi...@mailing.copernik.eu>.
Hi Gary,
On 16.11.2024 17:41, Gary Gregory wrote:
> [1]https://github.com/apache/logging-log4j2/issues/3143
>
> This issue points to the merged PR
> https://github.com/apache/logging-log4j2/pull/2936, a large change set with
> zero tests. This isn't great since there are no tests to catch regressions
> :-(
When PR #2936 was reviewed, bug #3143 wasn't known.
While PR #2936 (for the `2.25.x` release) is not affected by the bug,
the bug itself was caused by a backport of PR #2936 to the `2.24.x`
branch (see PR #2961[1]). The bug itself is due to the reachability
rules for Java local variables: a variable becomes unreachable **not**
at the end of the syntactic scope, but right after its last
**non-trivial** usage (see [2] for more details). That is not an easy
problem to catch in a review IMHO and certainly not something you can
easily test.
[1] https://github.com/apache/logging-log4j2/pull/2961
[2] https://shipilev.net/jvm/anatomy-quarks/8-local-var-reachability/
> [2]https://github.com/apache/logging-log4j2/pull/3209
>
> LGTM: If I apply the patch, the test is green, then when I revert the
> 'main' changes, the test fails. Nice work :-)
>
> [3]https://github.com/apache/logging-log4j2/pull/3210
>
> LGTM: If I apply the patch, the 2 tests are green, then when I revert the
> 'main' changes, the 2 tests fail. Nice work :-)
>
> So it all looks good :-)
Could you formally approve those PRs? Thanks.
Piotr
Re: Log4j release 2.24.1
Posted by Gary Gregory <ga...@gmail.com>.
Hi All,
Thank you Piotr for managing this release.
[1] https://github.com/apache/logging-log4j2/issues/3143
This issue points to the merged PR
https://github.com/apache/logging-log4j2/pull/2936, a large change set with
zero tests. This isn't great since there are no tests to catch regressions
:-(
[2] https://github.com/apache/logging-log4j2/pull/3209
LGTM: If I apply the patch, the test is green, then when I revert the
'main' changes, the test fails. Nice work :-)
[3] https://github.com/apache/logging-log4j2/pull/3210
LGTM: If I apply the patch, the 2 tests are green, then when I revert the
'main' changes, the 2 tests fail. Nice work :-)
So it all looks good :-)
HTH,
Gary
On Sat, Nov 16, 2024 at 4:35 AM Piotr P. Karwasz <pi...@mailing.copernik.eu>
wrote:
> Hi all,
>
> Due to bug #3143[1], which allows a `Logger` to be garbage-collected
> before it is returned by `LoggerContext#getLogger()`, I am planning a
> 2.24.2 release with just two changes:
>
> * A fix for the above mentioned bug (see PR #3209[2]).
>
> * A fix for a bug related to `ThreadContextMap` (see PR #3210[3]).
>
> Could you check those PRs to make sure we don't miss anything this time?
> I am planning the release for Sunday evening or Monday morning, but I'll
> wait until those PRs have reviews.
>
> Piotr
>
> [1] https://github.com/apache/logging-log4j2/issues/3143
>
> [2] https://github.com/apache/logging-log4j2/pull/3209
>
> [3] https://github.com/apache/logging-log4j2/pull/3210
>
>