Re: Possible Leak in MyFaces Orchestrea
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 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 > To: MyFaces Discussion > 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 ConversationManagerSessionListen
Re: Possible Leak in MyFaces Orchestrea
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 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 To: MyFaces Discussion 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 > > To: > MyFaces Discussion > > Date: > 12/27/2008 03:59 AM > Subject: > Re: Possible Leak in MyFaces > O
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 > > To: > MyFaces Discussion > > 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 somethin
Re: Possible Leak in MyFaces Orchestrea
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 To: MyFaces Discussion 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
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
Possible Leak in MyFaces Orchestrea
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? Thanks, Steve Ronderos