Re: svn commit: r1301177 - /commons/proper/digester/trunk/core/src/main/java/org/apache/commons/digester3/binder/AbstractRulesModule.java

[sebb <se...@gmail.com>]

On 15 March 2012 20:09,  <si...@apache.org> wrote:
> Author: simonetripodi
> Date: Thu Mar 15 20:09:31 2012
> New Revision: 1301177
>
> URL: http://svn.apache.org/viewvc?rev=1301177&view=rev
> Log:
> potential fix for [DIGESTER-163]

If the problem is due to multi-threading issues, isn't that because
the same instance is being shared between threads, presumably on
purpose?

If so, using ThreadLocal may fix the immediate problem, but won't it
prevent the data being shared?

> Modified:
>    commons/proper/digester/trunk/core/src/main/java/org/apache/commons/digester3/binder/AbstractRulesModule.java
>
> Modified: commons/proper/digester/trunk/core/src/main/java/org/apache/commons/digester3/binder/AbstractRulesModule.java
> URL: http://svn.apache.org/viewvc/commons/proper/digester/trunk/core/src/main/java/org/apache/commons/digester3/binder/AbstractRulesModule.java?rev=1301177&r1=1301176&r2=1301177&view=diff
> ==============================================================================
> --- commons/proper/digester/trunk/core/src/main/java/org/apache/commons/digester3/binder/AbstractRulesModule.java (original)
> +++ commons/proper/digester/trunk/core/src/main/java/org/apache/commons/digester3/binder/AbstractRulesModule.java Thu Mar 15 20:09:31 2012
> @@ -28,26 +28,26 @@ public abstract class AbstractRulesModul
>     implements RulesModule
>  {
>
> -    private RulesBinder rulesBinder;
> +    private final ThreadLocal<RulesBinder> rulesBinders = new ThreadLocal<RulesBinder>();
>
>     /**
>      * {@inheritDoc}
>      */
>     public final void configure( RulesBinder rulesBinder )
>     {
> -        if ( this.rulesBinder != null )
> +        if ( rulesBinders.get() != null )
>         {
>             throw new IllegalStateException( "Re-entry is not allowed." );
>         }
>
> -        this.rulesBinder = rulesBinder;
> +        rulesBinders.set( rulesBinder );
>         try
>         {
>             configure();
>         }
>         finally
>         {
> -            this.rulesBinder = null;
> +            rulesBinders.remove();
>         }
>     }
>
> @@ -68,7 +68,7 @@ public abstract class AbstractRulesModul
>      */
>     protected void addError( String messagePattern, Object... arguments )
>     {
> -        rulesBinder.addError( messagePattern, arguments );
> +        rulesBinders.get().addError( messagePattern, arguments );
>     }
>
>     /**
> @@ -80,7 +80,7 @@ public abstract class AbstractRulesModul
>      */
>     protected void addError( Throwable t )
>     {
> -        rulesBinder.addError( t );
> +        rulesBinders.get().addError( t );
>     }
>
>     /**
> @@ -91,7 +91,7 @@ public abstract class AbstractRulesModul
>      */
>     protected void install( RulesModule rulesModule )
>     {
> -        rulesBinder.install( rulesModule );
> +        rulesBinders.get().install( rulesModule );
>     }
>
>     /**
> @@ -103,7 +103,7 @@ public abstract class AbstractRulesModul
>      */
>     protected LinkedRuleBuilder forPattern( String pattern )
>     {
> -        return rulesBinder.forPattern( pattern );
> +        return rulesBinders.get().forPattern( pattern );
>     }
>
>     /**
> @@ -113,7 +113,7 @@ public abstract class AbstractRulesModul
>      */
>     protected RulesBinder rulesBinder()
>     {
> -        return rulesBinder;
> +        return rulesBinders.get();
>     }
>
>  }
>
>

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

Re: svn commit: r1301177 - /commons/proper/digester/trunk/core/src/main/java/org/apache/commons/digester3/binder/AbstractRulesModule.java

[Simone Tripodi <si...@apache.org>]

Hi Seb!

> If the problem is due to multi-threading issues, isn't that because
> the same instance is being shared between threads, presumably on
> purpose?
>

while it is known that the Digester is not thread-safety, the
DigesterLoader (that corresponds to its Factory, or Builder) shall
allow users creating different Digester instances without concurrency
issues: the loader describes the Digester rules only once, then will
create different Digester instances with same Rule types, but not same
Rule instances.

The user that submitted the issue reported his use-case where he
initializes the DigesterLoader when a Servlet starts, then create a
Digester for each request, it's here where the issue happens because
the Loader actually takes in consideration setting-up a Digester
instance at time, without locking or queuing the requests.

> If so, using ThreadLocal may fix the immediate problem, but won't it
> prevent the data being shared?

fortunately there is no need to share data across multiple threads,
the objects lyfecycle is very simple[1] and it is just a matter of
creating a DigesterLoader using a list of RulesModule, each of them
uses a RulesBinder to set new Rule instances to a new Digester
instance, when creating it on demand.

If you have suggestions, they will be more than welcome :)

thanks for reviewing, very appreciated!
best
-Simo

[1] http://commons.apache.org/digester/guide/binder.html

http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/

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