You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/02/16 14:49:18 UTC
[GitHub] [logging-log4j2] ppkarwasz opened a new pull request #756: [LOG4J2-3404] Creates default layouts using the available Configuration
ppkarwasz opened a new pull request #756:
URL: https://github.com/apache/logging-log4j2/pull/756
When a default `PatternLayout` is created without a `Configuration` a new `DefaultConfiguration` is created and abandoned. This causes a leak of `ConsoleManager`s.
This PR provides a `Configuration` argument whenever possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak merged pull request #756: [LOG4J2-3404] Creates default layouts using the available Configuration
Posted by GitBox <gi...@apache.org>.
carterkozak merged pull request #756:
URL: https://github.com/apache/logging-log4j2/pull/756
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] ppkarwasz commented on a change in pull request #756: [LOG4J2-3404] Creates default layouts using the available Configuration
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on a change in pull request #756:
URL: https://github.com/apache/logging-log4j2/pull/756#discussion_r808247913
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java
##########
@@ -763,10 +762,7 @@ public Builder withFooter(final String footer) {
@Override
public PatternLayout build() {
- // fall back to DefaultConfiguration
- if (configuration == null) {
- configuration = new DefaultConfiguration();
- }
+ // should work with a null configuration
Review comment:
This requires more explanations: the `PatternParser` does not use a configuration directly (or has null checks), but allows one to be injected into the `PatternConverter`s.
Looking at the available `PatternConverter`s, the color converters, "enc", "equals*", "highlight", "maxLength", "replace", "style", "ex*" require a configuration to create a `PatternParser`.
I didn't find any converters except `LiteralPatternConverter` that actually use `Configuration`. `LiteralPatternConverter` has appropriate null checks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #756: [LOG4J2-3404] Creates default layouts using the available Configuration
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #756:
URL: https://github.com/apache/logging-log4j2/pull/756#discussion_r808076590
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractManager.java
##########
@@ -204,6 +204,13 @@ protected static StatusLogger logger() {
return StatusLogger.getLogger();
}
+ /**
+ * For testing purposes.
+ */
+ protected static int getManagerCount() {
Review comment:
`protected static` is the same as `static` since static methods cannot be extended.
```suggestion
static int getManagerCount() {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] ppkarwasz commented on pull request #756: [LOG4J2-3404] Creates default layouts using the available Configuration
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on pull request #756:
URL: https://github.com/apache/logging-log4j2/pull/756#issuecomment-1041943565
@carterkozak: that should do it.
As far as I can see, no NPEs will result from providing `null` as configuration in the `PatternLayout`. I added a null check for the only case I found.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] ppkarwasz commented on pull request #756: [LOG4J2-3404] Creates default layouts using the available Configuration
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on pull request #756:
URL: https://github.com/apache/logging-log4j2/pull/756#issuecomment-1041594559
@carterkozak: I'll check if we can leave the `Configuration` as `null` and update the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org