New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Session scoped beans and activeViewMaps not cleaned up when PortletSession expires or is invalidated #2695
Comments
@javaserverfaces Commented |
@javaserverfaces Commented |
@javaserverfaces Commented
|
@javaserverfaces Commented |
@javaserverfaces Commented Can you see if you can use your own sessionDestroyed listener that gets invoked when the session gets destroyed and execute the following in it? For each portlet attribute: Then we should be able to trap the session attribute removed event and call the beanManager destroy method. Can you verify if that would work? |
@javaserverfaces Commented The problem I see is that this logic would need to replicated by every portlet bridge implementation. It would need to go into the portlet bridge spec. I'd rather see us go with Neil's solution because it will work with all portlet bridges that are in use today. We can address it at the spec level later. |
@javaserverfaces Commented Since these are Mojarra-managed session attributes, it would be nice if it could be handled by Mojarra. |
@javaserverfaces Commented And if that works, then we would proceed with the following: 1. File an issue to have the Portlet bridge spec be updated to state it needs to do this (to fix it at the bridge level). If it does not work, I will investigate further to see what needs to be done to get it working with the approach described earlier. |
@javaserverfaces Commented |
@javaserverfaces Commented Neil and Stan, I see your point that every portlet bridge impl will have to do the same thing. I counter by pointing out that every JSF impl will have to do the same thing if we go with simply applying your initial patch. I'm not opposed to applying the patch, but I do think that Manfred's suggestion deserves trying out. If it's not in the spec, it does seem better to keep it "closer to home", as Manfred's suggestion would provide for. |
@javaserverfaces Commented And in there you can listen for the event without having to require the end developer to declare a listener. This is the approach we've been using to bootstrap the JSF impl since 1.0. |
@javaserverfaces Commented All you have a SessionDestroyed time is the HttpSession with attributes and values from multiple portlets. I think the spec change that will be needed is to have something like WebappLifecycleListener defined in the spec as part of the JSF API. Then a bridge impl can extend or replace the default impl. I don't have the specifics worked out in my head though. |
@javaserverfaces Commented Thanks for the hint about the TLD. I will try it on the portlet bridge side and report back. |
@javaserverfaces Commented I tried the following steps using an HttpSessionListener with Liferay Faces Bridge. I tested with GlassFish 3.1.2, JBoss AS7, and Tomcat 7: For each portlet attribute: Good News:
Bad News:
Additional Thoughts:
Thanks for your help, Neil |
@javaserverfaces Commented The activeViewMaps should be empty after step #3 has runs its course with respect to all the portlet renamed managed session beans. Then once the session goes out of scope the reference to these maps should be garbage collected, so I do not think they are going to be an issue
Note you should not need to rename the 'com.sun.faces.activeViewMaps' attribute. |
@javaserverfaces Commented If the cleanup processing in WebappLifecycleListener.sessionDestroyed(HttpSessionEvent) method does not process the "com.sun.faces.activeViewMaps" attribute, then JBoss AS7 will experience a memory leak and hold onto references for @ViewScoped managed-beans. Stan can correct me, but I believe that the Mojarra BeanManager.destroy(String, Object) method must be called on each of the @ViewScoped managed-beans so that the JBoss InjectionProvider can have a chance to execute. If Mojarra were to perform cleanup processing for "com.sun.faces.activeViewMaps" in both WebappLifecycleListener.sessionDestroyed(HttpSessionEvent) and WebappLifecycleListener.attributeRemoved(HttpSessionBindingEvent), then we could implement your proposal in the bridge. I can post another patch if that would help. Thanks, Neil |
@javaserverfaces Commented OK with respect to those if you do not rename the com.sun.faces.application.view.activeViewMaps attribute at all, the regular processing will take care of cleaning up those references as part of the sessionDestroyed event so they would be covered. |
@javaserverfaces Commented The problem is that the Mojarra WebappLifecycleListener.sessionDestroyed(HttpSessionEvent) method tries to do this: Map<String, Object> activeViewMaps = (Map<String, Object>) session.getAttribute("com.sun.faces.application.view.activeViewMaps"); In a portlet environment, the attribute name is "javax.portlet.p.XXX?com.sun.faces.application.view.activeViewMaps" and so the value of activeViewMaps is null. Because it's null, the cleanup processing does not take place. Neil |
@javaserverfaces Commented |
@javaserverfaces Commented Understood – whether the bridge renames the attribute or not doesn't make a difference. The WebappLifecycleListener.sessionDestroyed(HttpSessionEvent) method always fails to perform cleanup processing in a portlet environment. Thanks, Neil |
@javaserverfaces Commented |
@javaserverfaces Commented |
@javaserverfaces Commented |
@javaserverfaces Commented |
@javaserverfaces Commented |
@javaserverfaces Commented |
@javaserverfaces Commented |
|
When running in the context of a request, Mojarra calls FacesContext.getCurrentInstance().getExternalContext.getSessionMap() in order to get/set session attributes. This provides the portlet bridge with the ability to get/set attributes using the PortletSession, which is a layer of abstraction on top of the HttpSession. But when a session expires, the com.sun.faces.application.WebappLifecycleListener.sessionDestroyed(HttpSessionEvent) method calls HttpSessionEvent.getSession() in order to get/set session attributes. This causes a memory leak when running in a portlet environment, because the portlet bridge is not consulted. Specifically, @SessionScoped managed-beans and the "com.sun.faces.activeViewMaps" attribute are not cleaned up.
The good news is that Section PLT.18.3 of the Portlet 2.0 Specification titled "Binding Attributes into a Session" requires that PortletSession attribute names be namespaced/prefixed with the "javax.portlet.p.?" pattern when they are stored in the underlying HttpSession. This would enable Mojarra to find the session attributes, so that cleanup can take place correctly during session expiration/invalidation.
Affected Versions
[2.1.17]
The text was updated successfully, but these errors were encountered: