openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Darimont (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (OPENJPA-2507) Weird EmptyStackException in CriteriaQueryImpl
Date Tue, 03 Jun 2014 12:05:02 GMT

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

Thomas Darimont commented on OPENJPA-2507:
------------------------------------------

Hello,

I just had a look at this and I think a solution that fixes this issue.

I noticed by debugging the provided example app that an instance of {{org.apache.openjpa.persistence.criteria.CriteriaQueryImpl}}
is shared between multiple instances of {{org.apache.openjpa.jdbc.kernel.JDBCStoreQuery}}.
In {{CriteriaQueryImpl}} the {{_contexts}} field is read and written by multiple threads.


I think the problem lies in {{org.apache.openjpa.persistence.criteria.CriteriaQueryImpl.getQueryExpressions(ExpressionFactory)}}:
{code:java}
   QueryExpressions getQueryExpressions(ExpressionFactory factory) {
        _contexts = new Stack<Context>();
        Context context = new Context(null, null, null);
        _contexts.push(context);
        return new CriteriaExpressionBuilder().getQueryExpressions(factory, this);
    }   
{code}
Since the (shared) {{_context}} field is modified and read by multiple threads this causes
the spurious NPE / EmptyStackExceptions to appear.
One (relatively easy) way to solve this would be to just wrap the {{_context}} in a {{ThreadLocal}}
- I added a patch file that does exactly that. Note that we remove the stored ThreadLocal
value when we leave {{getQueryExpressions}}

With this fixed I could run the provided example app without any problems. This patch would
work for openjpa 2.3.0 as well as 2.4.0.
I tested this with openjpa 2.3.0 directly. I created the patch from the latest svn head for
2.4.0.

Cheers,
Thomas

> Weird EmptyStackException in CriteriaQueryImpl
> ----------------------------------------------
>
>                 Key: OPENJPA-2507
>                 URL: https://issues.apache.org/jira/browse/OPENJPA-2507
>             Project: OpenJPA
>          Issue Type: Bug
>          Components: criteria
>    Affects Versions: 2.2.0, 2.3.0
>         Environment: openjpa 2.2.0 & 2.3.0, spring-data-jpa up to 1.4.2
>            Reporter: Ioannis Alexandrakis
>            Priority: Minor
>              Labels: criteria_api, openjpa
>         Attachments: exceptions.txt, openjpa-exception-demo.zip
>
>
> Using spring-data-jpa with openjpa, I sometimes encounter a strange error (not all the
times, but under heavy load it makes its appearance, sparsely though):
> ....
> Caused by: java.util.EmptyStackException
>         at java.util.Stack.peek(Stack.java:<arbitrary line>)
>         at org.apache.openjpa.persistence.criteria.CriteriaQueryImpl.ctx(CriteriaQueryImpl.java:<arbitrary
line>
> ....
> I do not know which behaviour triggers it, however I think it would be an improvement
to change (I did not know where exactly to file it, because it is both an improvement and
a bug in my opinion), in org.apache.openjpa.persistence.criteria.CriteriaQueryImpl, method:
Context ctx(), this:
> return _contexts == null || _contexts.isEmpty() ? null :  _contexts.peek();
> to something like this:
> try {
> 	return _contexts == null || _contexts.isEmpty() ? null :  _contexts.peek();
> } catch (EmptyStackException e) {
>     return null;
> }
> , in order to prevent a case where multiple threads modify the "_contexts" between the
evaluation of the inline "if".
> I am not able to reproduce it all the time, thus I can't create a useful test, neither
have I created a patch due to the simplicity of the 'fix'. However I believe it is a harmless
fix which could be considered a minor improvement.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message