You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Maurizio Cucchiara (Created) (JIRA)" <ji...@apache.org> on 2011/10/29 08:19:32 UTC
[jira] [Created] (OGNL-34) OgnlRuntime.getCompiler and
thread-safety.
OgnlRuntime.getCompiler and thread-safety.
------------------------------------------
Key: OGNL-34
URL: https://issues.apache.org/jira/browse/OGNL-34
Project: OGNL
Issue Type: Bug
Reporter: Maurizio Cucchiara
Priority: Minor
As you can see, {{getCompiler}} is not thread safe.
I recently added a new performance benchmark to test its 3d-safety and performance: during my tests I have experienced a fast execution on unsafe version vs the safe one (though every concurrent test instantiated a new compiler).
I have not yet investigated and I still don't know what can cause running more than one instance of the compiler in the same jvm. If necessary we can consider to make compiler a singleton in order to enforce this concept.
What do you think guys?
{code}
public static OgnlExpressionCompiler getCompiler( OgnlContext ognlContext )
{
if ( _compiler == null )
{
try
{
OgnlRuntime.classForName( ognlContext, "javassist.ClassPool" );
_compiler = new ExpressionCompiler();
}
catch ( ClassNotFoundException e )
{
throw new IllegalArgumentException(
"Javassist library is missing in classpath! Please add missed dependency!", e );
}
}
return _compiler;
}
{code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-34) OgnlRuntime.getCompiler and
thread-safety.
Posted by "Adrian Crum (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OGNL-34?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13139169#comment-13139169 ]
Adrian Crum commented on OGNL-34:
---------------------------------
In the code you posted, more than one thread can enter the if block. That is how you get more than one instance of the compiler.
> OgnlRuntime.getCompiler and thread-safety.
> ------------------------------------------
>
> Key: OGNL-34
> URL: https://issues.apache.org/jira/browse/OGNL-34
> Project: OGNL
> Issue Type: Bug
> Reporter: Maurizio Cucchiara
> Priority: Minor
> Labels: concurrency, thread-safety
>
> As you can see, {{getCompiler}} is not thread safe.
> I recently added a new performance benchmark to test its 3d-safety and performance: during my tests I have experienced a fast execution on unsafe version vs the safe one (though every concurrent test instantiated a new compiler).
> I have not yet investigated and I still don't know what can cause running more than one instance of the compiler in the same jvm. If necessary we can consider to make compiler a singleton in order to enforce this concept.
>
> What do you think guys?
> {code}
> public static OgnlExpressionCompiler getCompiler( OgnlContext ognlContext )
> {
> if ( _compiler == null )
> {
> try
> {
> OgnlRuntime.classForName( ognlContext, "javassist.ClassPool" );
> _compiler = new ExpressionCompiler();
> }
> catch ( ClassNotFoundException e )
> {
> throw new IllegalArgumentException(
> "Javassist library is missing in classpath! Please add missed dependency!", e );
> }
> }
> return _compiler;
> }
> {code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-34) OgnlRuntime.getCompiler and
thread-safety.
Posted by "Maurizio Cucchiara (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OGNL-34?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13139182#comment-13139182 ]
Maurizio Cucchiara commented on OGNL-34:
----------------------------------------
Hi Adrian,
yes I know, I try to express myself more clearly.
When I said:
{quote}
I still don't know what can cause running more than one instance of the compiler in the same jvm.
{quote}
I meant that I don't know the side-effects which can happen if there are more than one instance of the compiler.
> OgnlRuntime.getCompiler and thread-safety.
> ------------------------------------------
>
> Key: OGNL-34
> URL: https://issues.apache.org/jira/browse/OGNL-34
> Project: OGNL
> Issue Type: Bug
> Reporter: Maurizio Cucchiara
> Priority: Minor
> Labels: concurrency, thread-safety
>
> As you can see, {{getCompiler}} is not thread safe.
> I recently added a new performance benchmark to test its 3d-safety and performance: during my tests I have experienced a fast execution on unsafe version vs the safe one (though every concurrent test instantiated a new compiler).
> I have not yet investigated and I still don't know what can cause running more than one instance of the compiler in the same jvm. If necessary we can consider to make compiler a singleton in order to enforce this concept.
>
> What do you think guys?
> {code}
> public static OgnlExpressionCompiler getCompiler( OgnlContext ognlContext )
> {
> if ( _compiler == null )
> {
> try
> {
> OgnlRuntime.classForName( ognlContext, "javassist.ClassPool" );
> _compiler = new ExpressionCompiler();
> }
> catch ( ClassNotFoundException e )
> {
> throw new IllegalArgumentException(
> "Javassist library is missing in classpath! Please add missed dependency!", e );
> }
> }
> return _compiler;
> }
> {code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-34) OgnlRuntime.getCompiler and
thread-safety.
Posted by "Adrian Crum (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OGNL-34?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13139183#comment-13139183 ]
Adrian Crum commented on OGNL-34:
---------------------------------
Problems will occur when some threads try to use a partially-constructed instance of ExpressionCompiler.
> OgnlRuntime.getCompiler and thread-safety.
> ------------------------------------------
>
> Key: OGNL-34
> URL: https://issues.apache.org/jira/browse/OGNL-34
> Project: OGNL
> Issue Type: Bug
> Reporter: Maurizio Cucchiara
> Priority: Minor
> Labels: concurrency, thread-safety
>
> As you can see, {{getCompiler}} is not thread safe.
> I recently added a new performance benchmark to test its 3d-safety and performance: during my tests I have experienced a fast execution on unsafe version vs the safe one (though every concurrent test instantiated a new compiler).
> I have not yet investigated and I still don't know what can cause running more than one instance of the compiler in the same jvm. If necessary we can consider to make compiler a singleton in order to enforce this concept.
>
> What do you think guys?
> {code}
> public static OgnlExpressionCompiler getCompiler( OgnlContext ognlContext )
> {
> if ( _compiler == null )
> {
> try
> {
> OgnlRuntime.classForName( ognlContext, "javassist.ClassPool" );
> _compiler = new ExpressionCompiler();
> }
> catch ( ClassNotFoundException e )
> {
> throw new IllegalArgumentException(
> "Javassist library is missing in classpath! Please add missed dependency!", e );
> }
> }
> return _compiler;
> }
> {code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OGNL-34) OgnlRuntime.getCompiler and
thread-safety.
Posted by "Adrian Cumiskey (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OGNL-34?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13139445#comment-13139445 ]
Adrian Cumiskey commented on OGNL-34:
-------------------------------------
I think making ExpressionCompiler a singleton is a good idea. If we did decide to do this then we should also consider refactoring ExpressionCompiler as it currently has many static methods - perhaps migrating all of them to a companion ExpressionCompilerUtil.
I see two other options...
A) We take a small performance hit and wrap _compiler in a synchronized block.
B) We choose to not use the ClassResolver held by the OgnlContext but instead use OgnlContext.DEFAULT_CLASS_RESOLVER for resolving. Then there wouldn't be any need to synchronize access to _compiler because the resolution wouldn't be dependent on OgnlContext, we could instantiate ExpressionCompiler immediately. I think it would be a rare case that a custom class resolver would be required to ensure that javaassist is available.
Thoughts?
> OgnlRuntime.getCompiler and thread-safety.
> ------------------------------------------
>
> Key: OGNL-34
> URL: https://issues.apache.org/jira/browse/OGNL-34
> Project: OGNL
> Issue Type: Bug
> Reporter: Maurizio Cucchiara
> Priority: Minor
> Labels: concurrency, thread-safety
>
> As you can see, {{getCompiler}} is not thread safe.
> I recently added a new performance benchmark to test its 3d-safety and performance: during my tests I have experienced a fast execution on unsafe version vs the safe one (though every concurrent test instantiated a new compiler).
> I have not yet investigated and I still don't know what can cause running more than one instance of the compiler in the same jvm. If necessary we can consider to make compiler a singleton in order to enforce this concept.
>
> What do you think guys?
> {code}
> public static OgnlExpressionCompiler getCompiler( OgnlContext ognlContext )
> {
> if ( _compiler == null )
> {
> try
> {
> OgnlRuntime.classForName( ognlContext, "javassist.ClassPool" );
> _compiler = new ExpressionCompiler();
> }
> catch ( ClassNotFoundException e )
> {
> throw new IllegalArgumentException(
> "Javassist library is missing in classpath! Please add missed dependency!", e );
> }
> }
> return _compiler;
> }
> {code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira