logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Scott Harrington (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (LOG4J2-745) Plugins can cause ConverterKeys collisions with unpredictable results
Date Mon, 11 Aug 2014 02:18:12 GMT

     [ https://issues.apache.org/jira/browse/LOG4J2-745?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Scott Harrington updated LOG4J2-745:
------------------------------------

    Attachment: LOG4J2-745-patch-r1617171.txt

Hi Matt, thanks for taking a look.

1. I've rebased the patch to r1617171.

2. Fixed. Though as a user, the whole "serialized plugin listing files"
concept went right over my head until I realized that a Log4j2Plugins.dat 
existed and that's what it was for. Now that you've released it, you're
going to have to support it for some time, since third parties are going
to ship their JARs with a dat file and should reasonably expect their
plugins to be found by future Log4j 2.x point releases.

5. Fixed.

6. Remko fixed the empty-package-name problem, but your r1617171
revision re-introduces it. I've re-fixed it in my patch. Please follow
closely what happens if an empty string appears in the "packages"
attribute -- {{String.split()}} returns an array containing a single
empty string, which you then added to PACKAGES which then flows into
the {{Resolver.findInPackage} method which scans the whole CLASSPATH
looking for that empty package.

7. This is intentional. Before you only had the single static
{{PluginManager.PACKAGES}} list, but now each {{Configuration}} instance
also contains a list of plugin packages. This is necessary to allow
a Configuration (including its "packages" attribute) to be changed
at runtime, or for multiple Configurations with different "packages"
lists to co-exist within a JVM. For the benefit of anyone reading JIRA
but not studying my patch, here is my proposed change / clarification
to the plugin loading order and its documentation:

{quote}
... The PluginManager locates plugins by looking in four places:
# Serialized plugin listing files on the classpath. These files are 
generated automatically during the build (more details below).
# A comma-separated list of packages specified by the 
log4j.plugin.packages system property.
# Packages passed to the static PluginManager.addPackages method (before 
Log4j configuration occurs).
# The "packages" declared in your log4j2 configuration file.
{quote}


> Plugins can cause ConverterKeys collisions with unpredictable results
> ---------------------------------------------------------------------
>
>                 Key: LOG4J2-745
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-745
>             Project: Log4j 2
>          Issue Type: Improvement
>            Reporter: Scott Harrington
>            Assignee: Matt Sicker
>            Priority: Minor
>         Attachments: LOG4J2-745-patch-r1617171.txt, LOG4J2-745-patch.txt
>
>
> If I create a Converter plugin with ConverterKeys of "d" or "m" then there will be a
collision with the built-in DatePatternConverter or MessagePatternConverter.
> It is unpredictable which plugin gets used.
> I see two resolutions:
> (1) detect collisions in PatternParser and emit a warning so we know which implementation
will be used
> (2) use whichever Log4j2Plugins.dat appeared first in the CLASSPATH
> Predictable iteration order is usually accomplished by replacing HashMaps with LinkedHashMaps.
Could easily do this for thie PluginManager.plugins field. But PluginRegistry uses a ConcurrentHashMap.
> Is there a good reason to use ConcurrentHashMaps in PluginRegistry? It doesn't really
give you any concurrency -- a caller to PluginManager.getPlugins could see a partially-loaded
map if collectPlugins was still running. Why not synchronize collectPlugins and/or loadPlugins,
and force any concurrent caller to getPlugins to wait until the loading was complete.
> I would give it a stab but I see other more important changes are probably underway for
LOG4J2-741 and LOG4J2-673 and this can probably wait.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message