[jboss-jira] [JBoss JIRA] Commented: (JBRULES-2570) KnowledgeAgent lacks a safe disposal method
[
https://jira.jboss.org/browse/JBRULES-2570?page=com.atlassian.jira.plugin...
]
David Ward commented on JBRULES-2570:
-------------------------------------
Thanks for implementing dispose().
However, IMO, it would be nice to overload dispose() with dispose(boolean). By default,
dipose() would call dispose(false). However, if a user called dispose(true), then instead
of throwing an exception if there were any non-disposed sessions in the kbase's rbase,
it would instead iterate over the sessions and dipose() them itself. A clear warning of
"use this at your own risk" could be on the dispose(boolean) method, mentioning
the thread-unsafeness of sessions, and you better be sure you're really done with them
elsewhere in your running system.
Additionally, while perusing the code, I noticed 2 things:
1) I don't think a user should have to cast KnowledgeBase to KnowledgeBaseImpl to call
getRuleBase(). It should be on the KnowledgeBase interface.
2) KnowledgeBaseImpl provides a public getRuleBase() method, however it's ruleBase
member variable is also public. It should be private.
KnowledgeAgent lacks a safe disposal method
-------------------------------------------
Key: JBRULES-2570
URL: https://jira.jboss.org/browse/JBRULES-2570
Project: Drools
Issue Type: Bug
Security Level: Public(Everyone can see)
Components: drools-core (expert)
Affects Versions: 5.1.0.M2
Reporter: David Ward
Assignee: Esteban Aliverti
Priority: Critical
Fix For: 5.1.0.CR1
In the old RuleAgent api, there were a couple methods exposed that provided a safe
disposal/cleanup mechanism:
> ragent.stopPolling();
> for (StatefulSession s : ragent.getRuleBase().getStatefulSessions()) {
s.dispose(); }
In the new KnowledgeAgent api, there is only:
> kagent.monitorResourceChangeEvents(false);
This is problematic for a few reasons:
1. Calling kagent.monitorResourceChangeEvents(false) will stop the thread that reads
changesets from the queue and processes them, but the kagent will remain as a notifier
listener. So the kagent will continue putting a changeset on its queue whenever the
scanner detects a change, the result of this being a memory leak.
2. Without a way to get at the KnowledgeBase and it's StatefulKnowledgeSessions,
there is no way to cleanly dispose them.
3. There is currently no way for the ResourceChangeScannerService to know that it
should stop "watching" a resource, because it doesn't know when there are no
more listeners registered with the ResourceChangeNotifierService. The result of this is
unnecessary overhead.
To solve #1 and #2 above, a dispose() method should be added to KnowledgeAgent that
will:
a. Remove itself as a notifier listener.
b. THEN call monitorResourceChangeEvents(false) to stop it's internal thread.
c. Clear it's internal queue.
d. Iterate over any StatefulKnowledgeSessions associated with it's KnowledgeBase,
and dispose() them.
It could be argued that #3 above should be a separate Jira issue, but it is related to
this topic, so it deserves mentioning.
Notes:
1. This issue is a dependency for JBoss.org ESB 4.9, which is a dependency for
JBoss.com SOA-P 5.1. The dependent Jira issue is: JBESB-3360.
2. This issue has been marked as Critical, although I would consider it a Blocker.
However, I will leave that decision up to the Drools team. ;)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
https://jira.jboss.org/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira