Hi Steve, I've created a bugreport for Orchestra to track this: http://issues.apache.org/jira/browse/ORCHESTRA-35
I think your suggestion of using the HttpSessionListener interface is a good one. Unless someone objects, I will update Orchestra's ConversationManagerSessionListener to implement this interface and handle sessionDestroyed directly rather than relying on the container. I can't see how it could cause problems for other containers. Iterating over the attrs is a minor performance hit for apps with very large numbers of http-session attributes, but if the webpp is so complex then this loop is unlikely to be significant. I will try to find time to do this in the next few days. There are a couple of other minor Orchestra fixes that it would be nice to get addressed, so it is probably time to look at getting an orchestra-3.1 release out in the next month or so. Thanks for your very clear problem report.. Regards, Simon Steve Ronderos schrieb: > > 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 <skitch...@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 > > <skitch...@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 > > > > > > > > >