myfaces-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Steve Ronderos <steve.ronde...@ni.com>
Subject Re: Possible Leak in MyFaces Orchestrea
Date Wed, 31 Dec 2008 20:33:48 GMT
Simon,

Sorry about the top-post I wasn't thinking (and my email client is a 
piece...).

I too looked around for the spec on what should happen when a session 
times out.  We are using OC4J :-( .  I'll file a bug report with them. 

There was a workaround I was toying with that could be added to Orchestra 
if you think it is valuable.

I was essentially going to add an HttpSessionListener that removes 
attributes when the session times out.

public class AttributeRemovalSessionListener implements 
HttpSessionListener {

  public void sessionCreated(HttpSessionEvent se) {}

  public void sessionDestroyed(HttpSessionEvent se) {
    Enumeration<String> e = se.getSession().getAttributeNames();
    while (e.hasMoreElements()) {
      se.getSession().removeAttribute(e.nextElement());
    }
  }
}

With proper exception handling I believe that these methods and the 
HttpSessionListener interface could instead be added to the 
ConversationManagerSessionListener.  I have tested the above listener in 
OC4J and it works, if you think it is worth looking into I could test it 
out on some other Containers to make sure that it doesn't make a mess of 
things. 

Do you think this kind of solution is worth investigating?  Otherwise I 
can look at other workarounds.

Thanks,

Steve Ronderos



From:
Simon Kitching <skitching@apache.org>
To:
MyFaces Discussion <users@myfaces.apache.org>
Date:
12/31/2008 10:03 AM
Subject:
Re: Possible Leak in MyFaces Orchestrea



Hi Steve,

First, PLEASE do not top-post (ie put your reply at the top of an email)
when someone has previously used bottom-posting. It is really annoying
and makes the email almost impossible to read sensibly.
  See:  http://en.wikipedia.org/wiki/Posting_style

I've double-checked the servlet spec, and while I can't find explicit
wording that says that session timeout triggers removeAttribute on all
top-level attributes of the session, the docs here do imply it:
http://java.sun.com/javaee/5/docs/api/javax/servlet/http/HttpSessionBindingListener.html


Apache Tomcat is the servlet engine I use mostly, and it certainly does
do this. So a bugreport to your servlet-container vendor is probably a
good idea.

I'm happy to add a workaround in orchestra for this problem, though, if
we can find one. I don't see how adding HttpSessionBindingListener to
ConversationManager will help though; that will mean the
ConversationManager needs to be able to obtain a reference to the
ConversationManagerSessionListener which is not easily done.

Possibly the ConversationManagerSessionListener could add a dummy object
into each session, and this dummy object can then implement
HttpSessionBindingListener and contain a reference to the
ConversationManagerSessionListener. It probably needs to be transient
though (should't be distributed in clustered environments). And it
somehow also needs to handle session passivation/activation correctly.

If you can create a suitable patch for this issue, I would be happy to
review and apply it. Otherwise I'll try to find some time to come up
with a solution but it won't be for at least a few weeks.

By the way, what servlet container are you using (not that crappy
Websphere I hope; it's riddled with non-spec-compliant behaviour...)

Regards,
Simon

On Tue, 2008-12-30 at 10:43 -0600, Steve Ronderos wrote:
> 
> Simon, 
> 
> Thanks for responding! 
> 
> I didn't know about the ConversationManagerSessionListener, after
> poking at it for a little, I think I understand how it all works now.
>  Unfortunately I'm still experiencing the leak. 
> 
> I see in the Listener that it removes the ConversationManager objects
> in the method attributeRemoved.  Is it required for an HttpSession to
> remove it's attributes and therefore cause attributeRemoved to be
> called?  I believe the web container we are using does not cause
> attributeRemoved to be called.  Inside of the container we use when
> the session is invalidating the attributes are searched for instances
> of HttpSessionBindingListener.  Each instance that is found has its
> valueUnbound method called.  If I'm interpreting this correctly, that
> means that for ConversationManager objects not to leak in my
> container, they will need to implement this HttpSessionBindingListener
> interface and remove themselves from the ConversationWiperThread in
> the valueUnbound method.  Or the container would have to call
> removeAttribute when the session times out. 
> 
> Does this sound correct? 
> 
> At this point do you think it is an issue with my web container that I
> should file with the vendor? Or is this something that needs to change
> in Orchestra to accommodate the container. 
> 
> Thanks, 
> 
> Steve Ronderos 
> 
> 
> From: 
> Simon Kitching
> <skitching@apache.org> 
> To: 
> MyFaces Discussion
> <users@myfaces.apache.org> 
> Date: 
> 12/27/2008 03:59 AM 
> Subject: 
> Re: Possible Leak in MyFaces
> Orchestrea
> 
> 
> ______________________________________________________________________
> 
> 
> 
> On Tue, 2008-12-23 at 10:30 -0600, Steve Ronderos wrote:
> > 
> > Hello Orchestra Users,
> > 
> > I posted the following message to the developers mailing list a few
> > weeks ago and had no response. 
> > 
> > I was wondering if anyone has any information on a potential memory
> > leak that I see in Orchestra 1.2. 
> > 
> > It appears to me that conversationManagers in
> > ConversationWiperThread.java gets new ConversationManager objects
> > added to it but they are only removed through some serialization
> > method (I don't fully understand the distributed serialization stuff
> > since I have never used it).  I think that this leak is pretty
> > small... on the order of 10s of bytes per ConversationManager, but
> for
> > long lasting high traffic apps that could eventually become a
> problem.
> > Is there something that I have overlooked that ensures that these
> are
> > cleaned up? Is this an accepted shortcoming of Orchestra?  Should I
> > file a JIRA issue? 
> 
> Hi Steve,
> 
> I don't believe there is a problem here.
> 
> There is one ConversationManager instance per http-session. It gets
> created when needed (when something calls
> ConversationManager.getInstance) and is deleted when the http-session
> gets deleted.
> 
> There is one ConversationWiperThread instance per webapp. It never
> creates or destroys ConversationManager instances. However it does
> peek
> inside them in order to destroy Conversation and ConversationContext
> objects that timeout.
> 
> Unfortunately, there is no javax.servlet api for getting a list of all
> the HttpSession objects. Therefore in order for the
> ConversationWiperThread to know what ConversationManager objects
> exist,
> the ConversationManagerSessionListener class registers itself as a
> SessionListener object on the webapp, and detects when a
> ConversationManager object is added to an HttpSession.
> 
> The ConversationWiperThread does add these references into a non-weak
> map, so if they were never removed from the map that would indeed be a
> leak - when the http session is destroyed there would still be a
> reference to the ConversationManager.
> 
> However if you look at ConversationManagerSessionListener you will see
> that when an http session is destroyed, the ConversationManager
> instance
> (if any) in that session is removed from the wiper-thread's map.
> 
> So as far as I know, there is no leak.
> 
> There is also one other issue: the container can "passivate" a session
> (write it out to disk to free up memory). In this case, we also need
> to
> remove the ConversationManager from the wiper-thread. The
> sessionWillPassivate and sessionDidActivate methods in
> ConversationManagerSessionListener should handle that case too.
> 
> If you see any other way in which a memory leak can occur, please let
> us
> know.
> 
> Regards,
> Simon
> 
> 
> 




Mime
View raw message