[
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.txt
Matt: Collections.unmodifiableMap would only prevent the caller from modifying
the map, it would not prevent concurrent threads from modifying the map out
from under them.
I took a stab at a revised PluginRegistry / PluginManager based on my
observations above
* currently fails many tests - just posting so you can take a look (after your
2.0.1 release!)
* logs collisions at INFO level
* uses ConcurrentMap for thread-safety in the outer by-category and by-package
and by-loaderName maps (see comments)
* uses LinkedHashMaps in the inner (single-category) maps to preserve loading
order for collision detection; these are never modified after they are stored
in the outer map so are safe to return to callers of PluginManager.getPlugins
Also, PatternParser now logs if multiple plugins have conflicting
ConverterKeys, and only uses the plugin that appeared first in the loading
sequence.
> 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.txt, LinkedHashMap_and_locks.patch
>
>
> 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: [email protected]
For additional commands, e-mail: [email protected]
