[ 
https://issues.apache.org/jira/browse/LOG4J2-745?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14078271#comment-14078271
 ] 

Scott Harrington commented on LOG4J2-745:
-----------------------------------------

Matt: I looked over your locks and like the ReadWriteLock idea but I think we'd 
need to use some copy-on-write so that getPlugins can return a Map that can be 
safely iterated outside the lock; also you'd need to lock the other methods 
that mutate 'plugins' not just the ones that mutate REGISTRY.

I have some other ideas after looking at the code and following the discussion 
of "packages" at LOG4J2-741.

* The global / static REGISTRY -- which holds all plugins of all categories 
("core", "converter", "fileconverter", "level", "configurationfactory", 
"lookup") -- needs to be able to include duplicates that have the same name, 
and needs to store information about whether each plugin came from the 
log4j-core DAT cache, from a user-supplied DAT cache (and its order in the 
CLASSPATH), or from a specific package loaded at runtime.
* The PACKAGES list should not be global / static, but rather should be local 
to a Configuration (but shared across all categories)
* Each per-category PluginManager instance (owned by a particular 
Configuration) needs to scan the REGISTRY for just the plugins that match the 
category and that come from either (a) the log4j-core DAT cache, (b) a 
user-supplied DAT cache, or (c) the "packages" requested in the current 
configuration file (warning/resolving collisions in that specific order)
* Configuration "reconfigure" action should not modify the REGISTRY, but should 
just repeat the scan, possibly resolving collisions differently since the user 
might have modified the order of "packages" in their configuration file.

Needless to say this should wait til after 2.0.1 release.

> 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: 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: log4j-dev-unsubscr...@logging.apache.org
For additional commands, e-mail: log4j-dev-h...@logging.apache.org

Reply via email to