[jboss-jira] [JBoss JIRA] Commented: (JBRULES-2570) KnowledgeAgent lacks a safe disposal method

[David Ward (JIRA)]

    [ https://jira.jboss.org/browse/JBRULES-2570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12539493#action_12539493 ] 

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