Red Hat Bugzilla – Bug 782728
NPE when calling getProcessInstances()
Last modified: 2016-09-20 01:08:29 EDT
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
Every time, pull request will follow
Steps to Reproduce:
1. create persisted session
2. create/start process instance (optional)
3. call getProcessInstances()
Correct number of process instances or 0 if none was created yet.
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).
Added to pull request #49
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:
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
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.
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.
Please verify the issue on 5.3 ER4.
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.
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 ...
As long as this behaviour is properly documented, it can stay as it is.
Thank you Marco and Kris for suggestion about history log.
This behavior is actually already documented:
See http://docs.jboss.org/jbpm/v5.2/userguide/ch04.html#d0e671 (search for "getProcessInstances").
Included in product documentation as well, marking as verified.