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

Kevin Sutter commented on OPENJPA-2507:
---------------------------------------

Although I understand the logic behind your suggestion -- to prevent the 
EmptyStackException -- I don't necessarily agree with the reasoning for this 
change.  If multiple threads are getting into this method, then I think we have 
bigger issues with this class. None of the instance variables are protected for 
multiple thread access.  So, if we're having issues with the Stacks, we should 
be hitting issues with the Maps, and Lists, and other collections.

Given that, I also found an older JIRA (openjpa-2098) which seems to have hit a 
similar issue.  Pinaki had resolved that issue by putting in the null and 
isEmpty() checks that you are referencing.  So, it seems we are putting 
band-aids on this issue without fully understanding the source.

The common link between the two scenarios is the use of the spring-data-jpa 
feature.  Not pointing fingers at this, just highlighting the common aspect of 
the two descriptions.

> 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
>
> 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)

Reply via email to