You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by jasontedor <gi...@git.apache.org> on 2017/09/01 15:41:20 UTC
[GitHub] logging-log4j2 pull request #109: Fix stack locator calc location on JDK 9
GitHub user jasontedor opened a pull request:
https://github.com/apache/logging-log4j2/pull/109
Fix stack locator calc location on JDK 9
This commit fixes a bug in StackLocator#calcLocation on JDK 9. The
particular issue here is that on JDK 9, all stack trace locations report
as line 71 of StackLocatorUtil. This is due to a bug in the JDK 9
implementation of StackLocator. The bug is that instead of dropping the
top frames of the stack until the first frame that matches the
fully-qualified class name of the logger, the implementation would drop
all frames from the top that match the fully-qualified class name of the
logger. Of course, at this point in the stack trace, there would be
none. The fix is to reverse the condition, that we drop all frames until
we reach a frame matching the fully-qualified class name of the
logger. This commit also adds a test that was broken before this change
and now passes.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jasontedor/logging-log4j2 java-9-stack-locator-calc-location
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/logging-log4j2/pull/109.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #109
----
commit f87c6d11da8da299757eb582a16e7227a33a7b4b
Author: Jason Tedor <ja...@tedor.me>
Date: 2017-09-01T15:26:18Z
Fix stack locator calc location on JDK 9
This commit fixes a bug in StackLocator#calcLocation on JDK 9. The
particular issue here is that on JDK 9, all stack trace locations report
as line 71 of StackLocatorUtil. This is due to a bug in the JDK 9
implementation of StackLocator. The bug is that instead of dropping the
top frames of the stack until the first frame that matches the
fully-qualified class name of the logger, the implementation would drop
all frames from the top that match the fully-qualified class name of the
logger. Of course, at this point in the stack trace, there would be
none. The fix is to reverse the condition, that we drop all frames until
we reach a frame matching the fully-qualified class name of the
logger. This commit also adds a test that was broken before this change
and now passes.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by jasontedor <gi...@git.apache.org>.
Github user jasontedor commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
I can confirm that as well, both the unit tests we have that rely on this are passing now, and the application is logging the correct location when `%l` is used as a pattern.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by rgoers <gi...@git.apache.org>.
Github user rgoers commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
@jasontedor Thanks for contributing to Log4j - we appreciate all the help we can get!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by rgoers <gi...@git.apache.org>.
Github user rgoers commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
Finding the frame prior to the first frame that matches the class name of the logger is likely to return a frame that still points at the logger class because multiple methods in the logger may be called.
What the original code is trying to do is find the first stack frame that matches the fully-qualified class name of the logger and then skip all the stack frames for that logger. If it isn't doing that correctly then it obviously needs to be fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by jasontedor <gi...@git.apache.org>.
Github user jasontedor commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
Thanks @rgoers.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by jasontedor <gi...@git.apache.org>.
Github user jasontedor commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
This is for: https://issues.apache.org/jira/browse/LOG4J2-2028
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4j2 pull request #109: Fix stack locator calc location on JDK 9
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/logging-log4j2/pull/109
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by jasontedor <gi...@git.apache.org>.
Github user jasontedor commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
@MartyIX The bug report is linked [above](https://github.com/apache/logging-log4j2/pull/109#issuecomment-326626895). The bug manifests as that the source line on any logging event will appear to come from stack locator itself, which is clearly a mistake. This also impacts the use of the `%l` pattern. This bug only manifests on JDK 9.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by rgoers <gi...@git.apache.org>.
Github user rgoers commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
I just tried running one of my webapps on tomcat under Java 9. Although I didn't have much success due to Spring (that is a different story) I do see the location information correctly.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by jasontedor <gi...@git.apache.org>.
Github user jasontedor commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
You're welcome @rgoers, it is my pleasure. I pushed a commit as we discussed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by MartyIX <gi...@git.apache.org>.
Github user MartyIX commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
@jasontedor
> This is due to a bug in the JDK 9 implementation of StackLocator.
Do you happen to have a bug report link?
Thank you :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by jasontedor <gi...@git.apache.org>.
Github user jasontedor commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
@rgoers Thanks for the feedback, that makes sense! I will do this:
- make a change that will make that work
- add a comment to the code that explains why the code is doing what it's going to be doing
- update the test to account for that situation
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] logging-log4j2 issue #109: Fix stack locator calc location on JDK 9
Posted by MartyIX <gi...@git.apache.org>.
Github user MartyIX commented on the issue:
https://github.com/apache/logging-log4j2/pull/109
@jasontedor Does Log4j2 2.9.1 (with JDK 9) work correctly for you? All my log messages contain `calcLocation` method name. Am I the only one?
When I switch to JDK 8, it works as expected
---