Bug 782728 - NPE when calling getProcessInstances()
NPE when calling getProcessInstances()
Status: VERIFIED
Product: JBoss Enterprise BRMS Platform 5
Classification: JBoss
Component: jBPM 5 (Show other bugs)
BRMS 5.3.0.GA
Unspecified Unspecified
unspecified Severity urgent
: ---
: BRMS 5.3.0.GA
Assigned To: Marco Rietveld
Tomas Schlosser
: TestBlocker
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-18 04:46 EST by Tomas Schlosser
Modified: 2012-05-31 11:50 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Tomas Schlosser 2012-01-18 04:46:37 EST
Description of problem:
When calling getProcessInstances() on JPA persisted session, NullPointerException is thrown.

Version-Release number of selected component (if applicable):
jBPM master, BRMS 5.3.0.ER2

How reproducible:
Every time, pull request will follow

Steps to Reproduce:
1. create persisted session
2. create/start process instance (optional)
3. call getProcessInstances()
  
Actual results:
java.lang.NullPointerException
	at org.jbpm.persistence.processinstance.JPAProcessInstanceManager.getProcessInstances(JPAProcessInstanceManager.java:97)
	at org.jbpm.process.instance.ProcessRuntimeImpl.getProcessInstances(ProcessRuntimeImpl.java:200)
	at org.drools.common.AbstractWorkingMemory.getProcessInstances(AbstractWorkingMemory.java:1093)
	at org.drools.impl.StatefulKnowledgeSessionImpl.getProcessInstances(StatefulKnowledgeSessionImpl.java:292)
	at org.drools.command.runtime.process.GetProcessInstancesCommand.execute(GetProcessInstancesCommand.java:35)
	at org.drools.command.runtime.process.GetProcessInstancesCommand.execute(GetProcessInstancesCommand.java:28)
	at org.drools.command.impl.DefaultCommandService.execute(DefaultCommandService.java:36)
	at org.drools.persistence.SingleSessionCommandService.execute(SingleSessionCommandService.java:345)
	at org.drools.command.impl.CommandBasedStatefulKnowledgeSession.getProcessInstances(CommandBasedStatefulKnowledgeSession.java:139)
	at org.jbpm.persistence.session.GetProcessInstancesTest.testGetProcessInstances0(GetProcessInstancesTest.java:49)

Expected results:
Correct number of process instances or 0 if none was created yet.

Additional info:
It seems this problem is on jBPM master. However jBPM included in BRMS 5.3.0.ER2 returns 0 all the time (despite number of process instances started).
Comment 1 Tomas Schlosser 2012-01-18 04:55:37 EST
Added to pull request #49

https://github.com/droolsjbpm/jbpm/pull/49
Comment 2 Marco Rietveld 2012-01-20 20:22:24 EST
Part of the problems exposed by the pull request are easily fixed, but one of the main problems here is the following:

The JPAProcessInstanceManager is meant to keep track of ProcessInstances that are managed. 

Given that a persistent (stateful knowledge) session does all operations via a command that executes within a transaction -- and that at the end of the transaction the JPA PIM's process instance cache is emptied, the code in these tests _shouldn't_ work. 

The code in these tests expects that you can use ksession.getProcessInstances() to show how many process instances you have _created_. But what happens is the following: 

[ksession.createProcessInstance(...   ]
1. transaction is started (by the SSCS) 
2. process instance is created.
3. process instance is added to JPA PIM's (managed) process instance cache
4. transaction is ended
5. during which transaction synchronization runs and JPA PIM's process instance cache is emptied
[ksession.getProcessInstances()]
6. transaction is started
7. JPA PIM's proc inst cache is queried -- but it's empty. 
8. tx is ended

(I miss Jira's formatting capabilities!!! :( )

We could fix this -- but frankly, my vote is to "remove" or otherwise make the ksession.createProcessInstance a no-op. I don't see a valid use case for it -- why would users create processes, wait a while, and then start them, instead of just creating and starting them at the same time? 

There _may_ be cases in which the code is cleaner when we can create and then start processes, but locking down and regulating the API is a process and habit that will benefit us lots in the long run. 

---

_If_ we decide to fix this in the JPA PIM, then there are 2 (among probably many more) solutions I've thought of: 
- flag proc inst's in the cache and switch the flags (instead of cleaning the cache) on tx synchronization
- add a secondary "non-managed" cache to the JPA PIM and move proc inst's back and forth

Either way, we're faced with removing process instances that have finished -- possibly by including a mechanism in tx synchronization that will check if the process has finished and then remove it if it has.
Comment 3 Marco Rietveld 2012-01-26 07:53:35 EST
Tomas, 

The NPE has been fixed. 

The other problem (described above in my last comment) has been left as it is. 

Instead of merging your pull request via github, I just cherry-picked your commits and pushed them.
Comment 5 Ryan Zhang 2012-02-15 04:11:51 EST
Please verify the issue on 5.3 ER4.
Comment 6 Tomas Schlosser 2012-02-22 05:19:10 EST
As sent in mail I found more valid use cases:
- subprocess started as independent can't be signalled, aborted or queried for status
- restarting server and signalling or aborting process that was started previously

As I suggested in mail it would be great to either make the method work or add a method to retrieve the list of all possible IDs in database (as IDs are necessary for signalling/aborting a process).

I mark this as a test blocker as it is often necessary to retrieve the processes already started and persisted.
Comment 7 Marco Rietveld 2012-04-11 17:43:59 EDT
I e-mailed with Kris about this. Please read the comments and let me know what you think.

Below are Kris's comments: 

We can't change the implementation of the getProcessInstances() to retrieve all persisted instances, as (1) that conflicts with how we are using it now, to keep track of the managed instances and (2) isn't very useful either, as in real life cases, that would be a large list and you still cannot figure out which instance you actually need in that case.

There are multiple solutions / workarounds for this I think:
 * you can use the history or audit log, that contains info about all process instances
 * you can inspect the state of the process instance that is returned if you for example need a process instance id

Longer term, this is also related to process instance correlation, the ability to use a business key or some other data to find the process instance you need. But that is a feature that won't make it into BRMS 5.3.

I would recommend documenting this but not changing the current implementation.

-----------------------------------------------------
[Regarding adding a new method to retrieve the id's:]

I have the impression this would quickly extend to what the history log is offering.  Because what if the process instance you started already completed (you wouldn't be able to see it in this case, while the history log would allow you to query that as well)?  Or what if you want to start querying based on some variable value, or some other property.

I think they should either
 (a) inspect the process instance that is returned when starting a process to get the info they need
 (b) look at the history log for more complete information including other instances etc.

Not sure there is need for something in between ...
Comment 8 Tomas Schlosser 2012-04-18 06:12:12 EDT
As long as this behaviour is properly documented, it can stay as it is.

Thank you Marco and Kris for suggestion about history log.
Comment 11 Marco Rietveld 2012-04-30 14:42:12 EDT
Hi Tomas, 

This behavior is actually already documented: 

See http://docs.jboss.org/jbpm/v5.2/userguide/ch04.html#d0e671 (search for "getProcessInstances"). 


Thanks,
Marco
Comment 12 Tomas Schlosser 2012-05-31 11:50:25 EDT
Included in product documentation as well, marking as verified.

Note You need to log in before you can comment on or make changes to this bug.