commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Frank (JIRA)" <j...@apache.org>
Subject [jira] Commented: (JELLY-277) XMLParser.configure is not threadsafe.
Date Thu, 16 Apr 2009 14:06:14 GMT

    [ https://issues.apache.org/jira/browse/JELLY-277?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12699709#action_12699709
] 

Frank commented on JELLY-277:
-----------------------------

Since the variable jellyProperties is static making the method synchronized is not enough;
there will still be race conditions between different instances of XMLParser.

First it should be clear what the desired behaviour is:
 - is it essential that the properties are loaded only once, or
 - is it just a cache to avoid  processing each time

In the first case it is probably best to load the properties in a static block, through a
static synchronized method or in a block synchronized on a static instance.

private static Properties jellyProperties;

static {
   jellyProperties = xxx;
}

or

protected synchronized Properties getJellyProperties() {
 return _getJellyProperties();
}

protected static synchronized Properties _getJellyProperties() {
 if (jellyProperties == null) {...}
 ...
}

or

protected synchronized Properties getJellyProperties() {
 
synchronized (this.getClass()){
  if (jellyProperties == null) {...}
 ...
}


In the second case it is enough to first fill a local Properties object before assigning it
to the static variable; and not first assign it and then fill it because that corrupts the
test on the 'nullness' of the jellyProperties. This avoids any additional synchronization
but might get the properties to be filled in a number of times in parallel. As long as the
properties are never modified this should be enough.

 protected synchronized Properties getJellyProperties() {
        if (jellyProperties == null) {
            Properties tmpJellyProperties = new Properties();
            ...
            jellyProperties = tmpJellyProperties;
       }
       ...


> XMLParser.configure is not threadsafe.
> --------------------------------------
>
>                 Key: JELLY-277
>                 URL: https://issues.apache.org/jira/browse/JELLY-277
>             Project: Commons Jelly
>          Issue Type: Bug
>          Components: core / taglib.core
>    Affects Versions: 1.0
>         Environment: Linux 64bits, JDK build 1.6.0_01-b06
>            Reporter: Yung-Lin Ho
>
> XMLParser will try to load jelly.properties from disk when it first being used. However,
because XMLParser.configure method is not threadsafe, when I tried to run multiple jelly scripts/instance
at the same time, XMLParser threw out the following error message 
> java.lang.ClassNotFoundException: core
>     at org.apache.tools.ant.AntClassLoader.findClassInComponents(AntClassLoader.java:1166)
>     at org.apache.tools.ant.AntClassLoader.findClass(AntClassLoader.java:1107)
>     at org.apache.tools.ant.AntClassLoader.loadClass(AntClassLoader.java:977)
>     at java.lang.ClassLoader.loadClass(ClassLoader.java:251)
>     at org.apache.commons.jelly.parser.XMLParser.createTag(XMLParser.java:985)
> java.lang.ClassNotFoundException: core
>     at org.apache.commons.jelly.parser.XMLParser.createSAXException(XMLParser.java:1180)
>     at org.apache.commons.jelly.parser.XMLParser.createTag(XMLParser.java:990)
>     at org.apache.commons.jelly.parser.XMLParser.startElement(XMLParser.java:593)
> In order to fix this problem, we have to make ensureConfigured() and configure() synchronized
methods.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message