DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=31234>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=31234

Event Aware cache Registry memory leak?

           Summary: Event Aware cache Registry memory leak?
           Product: Cocoon 2
           Version: 2.1.5
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Severity: Normal
          Priority: Other
         Component: blocks
        AssignedTo: [EMAIL PROTECTED]
        ReportedBy: [EMAIL PROTECTED]
                CC: [EMAIL PROTECTED]


While testing the new event-aware cache component in our environment (one of the
cocoon blocks) I discovered that everytime I issued a clear-cache event and
reloaded the exact same content, the event registry got bigger and bigger with
duplicate event objects. (No other content was being loaded at the time.)

In EventAwareCacheImpl, the processEvent method is responsible for cleaning up
the event registry, as it calls the remove method. I assume that the intention
was for the local remove method to be invoked, but this never happens - the
parent remove method gets invoked instead! 

The reason is the signature of the remove method in EventAwareCacheImpl expects
(incorrectly, I assume) a PipelineCacheKey object to be passed in, but
processEvent never casts the key objects it is dealing with to PipelineCacheKey
 before calling remove(key) - ie. processEvent deals with Serializable object
classes, NOT PipelineCacheKey. 

You can see this on line 111, where the processEvent method retrieves the keys
to be removed:
     Serializable[] keys = m_eventRegistry.keysForEvent(e);
     ...
It then calls remove:
     remove(keys[i]);

The reason this doesn't cause a build error is EventAwareCacheImpl's parent
CacheImpl contains a remove method that matches the signature that processEvent
requires:

    public void remove(Serializable key) {
        if ( this.getLogger().isDebugEnabled()) {
            this.getLogger().debug("Removing cached response for " + key); 
        }
        this.store.remove(key);
    }

Of course, CacheImpl.remove only removes the item from the cache store. It
doesn't remove the relevant items from the event registry.

So, the end result is everything compiles and runs without error, but
keys/events never get removed from EventRegistry when processEvent is called
because it always invokes the parent remove method. So, the EventRegistry
balloons with duplicate key->event mappings.

To fix this, I simply changed the EventAwareCacheImpl remove method signature to
match its parent. Here's the change in unified diff format:

>svn diff EventAwareCacheImpl.java
Index: EventAwareCacheImpl.java
===================================================================
--- EventAwareCacheImpl.java    (revision 46070)
+++ EventAwareCacheImpl.java    (working copy)
@@ -36,7 +36,7 @@
  * in two MultiHashMap to facilitate efficient lookup by either as Key.
  *
  * @author Geoff Howard ([EMAIL PROTECTED])
- * @version $Id: EventAwareCacheImpl.java,v 1.9 2004/03/05 13:01:56 bdelacretaz
 Exp $
+ * @version $Id$
  */
 public class EventAwareCacheImpl extends CacheImpl implements Initializable,
                                                               EventAware {
@@ -95,7 +95,7 @@
      * Un-register this key in the EventRegistry in addition to
      * removing it from the Store
         */
-       public void remove(PipelineCacheKey key) {
+       public void remove(Serializable key) {
                super.remove(key);
         m_eventRegistry.removeKey(key);
        }

With this fix, the remove method in EventAwareCacheImpl is called, so the
registry tidied up as expected.

btw, I'm actually contemplating stopping processEvent from calling remove. It
looks to me like removing an item from the cache at this point prevents you from
taking advantage of the caching-point functionality. In
AbstractCachingProcessingPipeline.validatePipeline, it only tries finding a
cached item with a shorter key if the item __it found__ was invalid. 

If the item has been removed from the cache by processEvent, then it can never
subsequently be found by the processing pipeline and checked for validity. And
if validatePipeline fails to retrieve an item matching the full key, it gives up
on the pipeline completely. It only tries again with a shorter key if an item
was found, but it was invalid. Perhaps the processing pipeline should be
changed, rather than processEvent, to try a shorter key even when a cached item
for the full key was not found? Any comments?

Reply via email to