Issue Details (XML | Word | Printable)

Key: JAVASERVERFACES-2561
Type: New Feature New Feature
Status: Closed Closed
Resolution: Fixed
Priority: Critical Critical
Assignee: Manfred Riem
Reporter: Ed Burns
Votes: 2
Watchers: 3
Operations

If you were logged in you would be able to see more operations.
javaserverfaces

Implement requirements for @PreDestroy invocation upon session expiration on @ViewScoped managed beans

Created: 26/Oct/12 03:14 PM   Updated: 16/Sep/13 02:19 PM   Resolved: 29/Nov/12 05:40 PM
Component/s: lifecycle
Affects Version/s: 2.0, 2.1
Fix Version/s: 2.1.16, 2.2.0-m07

Time Tracking:
Original Estimate: 8 hours
Original Estimate - 8 hours
Remaining Estimate: 8 hours
Remaining Estimate - 8 hours
Time Spent: Not Specified
Time Spent - Not Specified

File Attachments: 1. Zip Archive 20121025-1757-i_spec_905-mods.zip (54 kB) 26/Oct/12 03:14 PM - Ed Burns
2. Text File 20121026-0924-i_spec_905.patch (49 kB) 26/Oct/12 03:14 PM - Ed Burns
3. Text File changebundle.txt (47 kB) 28/Nov/12 04:56 PM - Manfred Riem
4. Zip Archive newfiles.zip (28 kB) 28/Nov/12 04:56 PM - Manfred Riem

Issue Links:
Dependency
 
Related
 

Tags: JSF2_2
Participants: Ed Burns, facboy, Manfred Riem, ssilvert and tedgoddard


 Description  « Hide

See JAVASERVERFACES_SPEC_PUBLIC-905. Particularly see the new specification content in javax.faces.bean.ViewScoped.



Ed Burns added a comment - 26/Oct/12 03:30 PM

Stan said

We're also getting reports of memory leaks with view-scoped beans. I've verified the leak, but not traced its exact cause. I'm assuming that the leak is related to this issue.

Any opinion on that or insight into how the instances are referenced? I'm assuming that if view-scoped beans aren't eligible for GC when the session ends then they must be tied to application scope or to a ThreadLocal. Any other possibilities?

After partially implementing this, I can see where a leak happens, and I believe it will be fixed by this issue.


ssilvert added a comment - 06/Nov/12 09:01 PM - edited

The patch does fix the leak, but it can produce a NPE if the session has been invalidated and then it later times out.

BeanManager.getViewRootsWithViewScopedBeansDirectlyFromSession() doesn't take into account that httpSession.getAttribute() can return null. Just add a finally block like this:

public static List<UIViewRoot> getViewRootsWithViewScopedBeansDirectlyFromSession(HttpSession httpSession) {
         List<UIViewRoot>  result = null;
         try {
             result = (List<UIViewRoot>)httpSession.getAttribute(VIEWROOTS_WITH_VIEWSCOPED_BEANS_KEY);
         } catch (IllegalStateException ise) {
             if (LOGGER.isLoggable(Level.FINEST)) {
                 LOGGER.log(Level.FINEST, "Unable to obtain list of ViewScoped beans for clean up on session expiration", ise);
             }
         } finally {
             if (result == null) {
                 result = Collections.emptyList();
             }
         }
         return result;
     }

Manfred Riem added a comment - 29/Nov/12 04:39 PM

Applied to 2.1 branch,

svn commit -m "Fixed http://java.net/jira/browse/JAVASERVERFACES-2561, r=rogerk, push the ViewScope into the session, maintained by the JSF runtime, so it properly calls the constructor, @PostConstruct and @PreDestroy all only once."
Sending jsf-api\src\main\java\javax\faces\component\UIViewRoot.java
Sending jsf-ri\src\main\java\com\sun\faces\application\WebappLifecycleListener.java
Sending test\agnostic\pom.xml
Adding test\agnostic\scope
Adding test\agnostic\scope\pom.xml
Adding test\agnostic\scope\view
Adding test\agnostic\scope\view\pom.xml
Adding test\agnostic\scope\view\src
Adding test\agnostic\scope\view\src\main
Adding test\agnostic\scope\view\src\main\java
Adding test\agnostic\scope\view\src\main\java\com
Adding test\agnostic\scope\view\src\main\java\com\sun
Adding test\agnostic\scope\view\src\main\java\com\sun\faces
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view\ConstructorBean.java
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view\InvalidatedBean.java
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view\NavigateAwayBean.java
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view\NavigateBean.java
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view\PostConstructBean.java
Adding test\agnostic\scope\view\src\main\resources
Adding test\agnostic\scope\view\src\main\webapp
Adding test\agnostic\scope\view\src\main\webapp\WEB-INF
Adding test\agnostic\scope\view\src\main\webapp\WEB-INF\glassfish-web.xml
Adding test\agnostic\scope\view\src\main\webapp\WEB-INF\web.xml
Adding test\agnostic\scope\view\src\main\webapp\constructor.xhtml
Adding test\agnostic\scope\view\src\main\webapp\invalidatedPerform.xhtml
Adding test\agnostic\scope\view\src\main\webapp\invalidatedSession.xhtml
Adding test\agnostic\scope\view\src\main\webapp\invalidatedVerify.xhtml
Adding test\agnostic\scope\view\src\main\webapp\navigate.xhtml
Adding test\agnostic\scope\view\src\main\webapp\navigateAway.xhtml
Adding test\agnostic\scope\view\src\main\webapp\navigatedAway.xhtml
Adding test\agnostic\scope\view\src\main\webapp\postconstruct.xhtml
Adding test\agnostic\scope\view\src\test
Adding test\agnostic\scope\view\src\test\java
Adding test\agnostic\scope\view\src\test\java\com
Adding test\agnostic\scope\view\src\test\java\com\sun
Adding test\agnostic\scope\view\src\test\java\com\sun\faces
Adding test\agnostic\scope\view\src\test\java\com\sun\faces\test
Adding test\agnostic\scope\view\src\test\java\com\sun\faces\test\agnostic
Adding test\agnostic\scope\view\src\test\java\com\sun\faces\test\agnostic\scope
Adding test\agnostic\scope\view\src\test\java\com\sun\faces\test\agnostic\scope\view
Adding test\agnostic\scope\view\src\test\java\com\sun\faces\test\agnostic\scope\view\Issue2561IT.java
Transmitting file data .....................
Committed revision 11050.


Manfred Riem added a comment - 29/Nov/12 05:01 PM

Applied to 2.2 trunk,

svn commit -m "Fixed http://java.net/jira/browse/JAVASERVERFACES-2561, r=rogerk, push the ViewScope into the session, maintained by the JSF runtime, so it properly calls the constructor, @PostConstruct and @PreDestroy all only once."
Sending jsf-api\src\main\java\javax\faces\component\UIViewRoot.java
Sending jsf-ri\src\main\java\com\sun\faces\application\WebappLifecycleListener.java
Sending test\agnostic\pom.xml
Adding test\agnostic\scope
Adding test\agnostic\scope\pom.xml
Adding test\agnostic\scope\view
Adding test\agnostic\scope\view\pom.xml
Adding test\agnostic\scope\view\src
Adding test\agnostic\scope\view\src\main
Adding test\agnostic\scope\view\src\main\java
Adding test\agnostic\scope\view\src\main\java\com
Adding test\agnostic\scope\view\src\main\java\com\sun
Adding test\agnostic\scope\view\src\main\java\com\sun\faces
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view\ConstructorBean.java
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view\InvalidatedBean.java
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view\NavigateAwayBean.java
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view\NavigateBean.java
Adding test\agnostic\scope\view\src\main\java\com\sun\faces\test\agnostic\scope\view\PostConstructBean.java
Adding test\agnostic\scope\view\src\main\webapp
Adding test\agnostic\scope\view\src\main\webapp\WEB-INF
Adding test\agnostic\scope\view\src\main\webapp\WEB-INF\glassfish-web.xml
Adding test\agnostic\scope\view\src\main\webapp\WEB-INF\web.xml
Adding test\agnostic\scope\view\src\main\webapp\constructor.xhtml
Adding test\agnostic\scope\view\src\main\webapp\invalidatedPerform.xhtml
Adding test\agnostic\scope\view\src\main\webapp\invalidatedSession.xhtml
Adding test\agnostic\scope\view\src\main\webapp\invalidatedVerify.xhtml
Adding test\agnostic\scope\view\src\main\webapp\navigate.xhtml
Adding test\agnostic\scope\view\src\main\webapp\navigateAway.xhtml
Adding test\agnostic\scope\view\src\main\webapp\navigatedAway.xhtml
Adding test\agnostic\scope\view\src\main\webapp\postconstruct.xhtml
Adding test\agnostic\scope\view\src\test
Adding test\agnostic\scope\view\src\test\java
Adding test\agnostic\scope\view\src\test\java\com
Adding test\agnostic\scope\view\src\test\java\com\sun
Adding test\agnostic\scope\view\src\test\java\com\sun\faces
Adding test\agnostic\scope\view\src\test\java\com\sun\faces\test
Adding test\agnostic\scope\view\src\test\java\com\sun\faces\test\agnostic
Adding test\agnostic\scope\view\src\test\java\com\sun\faces\test\agnostic\scope
Adding test\agnostic\scope\view\src\test\java\com\sun\faces\test\agnostic\scope\view
Adding test\agnostic\scope\view\src\test\java\com\sun\faces\test\agnostic\scope\view\Issue2561IT.java
Transmitting file data .....................
Committed revision 11051.


tedgoddard added a comment - 11/Jan/13 10:36 PM

Now that the ViewScope is stored in the session, does this mean that ViewScope is never cleared until the session expires?


Manfred Riem added a comment - 14/Jan/13 02:18 PM

It should clear the view map once the user navigates away, or if the session expires. If that is not the behavior you are seeing please send us a reproducer so we can make sure we have not missed a corner case!


tedgoddard added a comment - 14/Jan/13 05:07 PM

This appears to leak for direct URLs visited in the application. One solution would be to implement a listener on LRUMap to remove the ViewScope when the component tree is removed.


facboy added a comment - 16/Sep/13 02:12 PM

the upshot of this seems to be that ViewScoped managed beans are always kept in the session now. We have an app that was relying on ViewScoped managed beans being serialized into the ViewState so they had no server-side state. Am I right in saying that this is no longer the case?


Manfred Riem added a comment - 16/Sep/13 02:19 PM

The ViewScoped beans are now in the session as the serialization did not follow <init> + @PostConstruct and @PreDestroy. Now the @ViewScoped beans are properly following this as any other bean. Note both @ViewScoped aka CDI and non-CDI are now consistent with each other. And yes you are correct.