Bug 779189 (SOA-1583)

Summary:	JbpmConfiguration.popJbpmConfiguration() doesn't implement a pop operation
Product:	[JBoss] JBoss Enterprise SOA Platform 4	Reporter:	Toshiya Kobayashi <tkobayas>
Component:	JBPM - within SOA, JBPM - standalone	Assignee:	Alejandro Guizar <alex.guizar>
Status:	CLOSED NEXTRELEASE	QA Contact:
Severity:	high	Docs Contact:
Priority:	high
Version:	4.3 CP02	CC:	dlesage, imamura.yousuke
Target Milestone:	---
Target Release:	FUTURE
Hardware:	Unspecified
OS:	Unspecified
URL:	http://jira.jboss.org/jira/browse/SOA-1583
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2010-05-26 01:11:35 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:

Description Toshiya Kobayashi 2009-11-06 05:01:55 UTC

Affects: Release Notes
Date of First Response: 2009-11-09 08:23:28
Help Desk Ticket Reference: https://enterprise.redhat.com/issue-tracker/362213
project_key: SOA

org.jbpm.JbpmConfiguration.java:
=====
synchronized void popJbpmConfiguration() {
  getJbpmConfigurationStack().remove(this);
}
=====

List.remove() can break ordering in the jbpmConfigurationStack, then eventually getCurrentJbpmConfiguration() will return an wrong jbpmConfiguration instance.
(List.remove(java.lang.Object) behaves like FIFO if there are multiple elements which refer the same object)

On the other hand, popJbpmContext() pops properly.

=====
void popJbpmContext(JbpmContext jbpmContext) {
  List stack = getJbpmContextStack();
  int size = stack.size();
  if (size == 0) {
...
  }
  else if (jbpmContext != stack.remove(size - 1)) {
...
  }
}
=====

Comment 1 Toshiya Kobayashi 2009-11-06 05:12:30 UTC

Link: Added: This issue incorporates JBPM-2630

Comment 2 Julian Coleman 2009-11-09 13:23:28 UTC

This is a candidate for SOA 4.3.0 CP03.

Comment 3 Alejandro Guizar 2009-12-03 04:08:52 UTC

Resolving platform issue as project peer JBPM-2630 has been resolved.

Comment 4 David Le Sage 2010-02-25 05:50:39 UTC

Please review the following draft text added to the Resolved Issues section of the Release Notes: 


https://jira.jboss.org/jira/browse/JBPM-2630 

    Previously, the JbpmConfiguration.popJbpmConfiguration() method did not implement 
    "pop" operations correctly. As a consequence, jBPM configurations were being removed from 
    thread-local lists incorrectly in a "First In, First Out" (FIFO) order. 

    To resolve this issue, org.jbpm.JbpmConfiguration.java has been changed. As a result, 
    jBPM configuration lists are now removed from the thread-local list in the correct LIFO order.

Comment 5 Toshiya Kobayashi 2010-02-25 06:28:24 UTC

> As a consequence, jBPM configurations were being removed from
> thread-local lists incorrectly in a "First In, First Out" (FIFO) order. 

To be precise,
They were removed in FIFO order when there are multiple elements which refer the same object.

Thanks,

Comment 6 David Le Sage 2010-02-25 06:37:57 UTC

Thank you.  Your fast response is greatly appreciated.  I have amended the release notes as per your recommendation.

Comment 7 Martin Vecera 2010-03-24 11:03:39 UTC

popJbpmConfiguration() has been copied from context pop including comments that are invalid for configurations: 
==== 
log.warn(this + " was not found in thread-local stack;" + "do not access JbpmContext instances from multiple threads"); 
==== 
log.warn(this + " was not closed in reverse creation order;" + " check your try-finally clauses around JbpmContext blocks"); 
====

Comment 8 Martin Vecera 2010-03-31 06:44:45 UTC

In the previous comment to this JIRA I used a word "comments" that is misleading. "log messages" would have been better.

Comment 11 Alejandro Guizar 2010-04-07 00:23:50 UTC

The log messages are valid. Each time a JbpmContext is pushed/popped, the corresponding JbpmConfiguration is also pushed/popped so that method JbpmContext.getCurrentJbpmContext() can be implemented. There is no other scenario for pushing/popping JbpmConfigurations. If a JbpmConfiguration is not present in the thread-local stack, this is most likely because the JbpmContext was closed in a thread where it wasn't created, hence the warning. JbpmContext is not designed for access from multiple threads.

Since no change is needed, can this issue still go into CP03?

Comment 12 Alejandro Guizar 2010-05-26 01:11:25 UTC

As requested by Martin V, I reviewed and improved the warnings logged when closing a JbpmContext in unstructured manner or from another thread.
Additionally, I wrote a test to check that unstructured JbpmContext close() operations succeed despite the warning.