Details
Bug
Major
Description
When PersistentFacesState.redirectTo() is invoked in a push application (such as when added to auctionMonitor with a Thread.sleep() added to one of the getters), the application can fail with a NullPointerException in various places, such as:
javax.faces.FacesException: java.lang.NullPointerException
at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:128)
at com.sun.faces.lifecycle.LifecycleImpl.render(LifecycleImpl.java:139)
at com.icesoft.faces.webapp.xmlhttp.PersistentFacesState.render(PersistentFacesState.java:176)
at com.icesoft.faces.webapp.xmlhttp.PersistentFacesState.executeAndRender(PersistentFacesState.java:312)
at com.icesoft.faces.async.render.RunnableRender.run(RunnableRender.java:143)
at edu.emory.mathcs.backport.java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:665)
at edu.emory.mathcs.backport.java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:690)
at java.lang.Thread.run(Thread.java:637)
Caused by: java.lang.NullPointerException
at com.icesoft.faces.application.D2DViewHandler.renderResponse(D2DViewHandler.java:329)
at com.icesoft.faces.application.D2DViewHandler.renderView(D2DViewHandler.java:159)
at com.sun.faces.lifecycle.RenderResponsePhase.execute(RenderResponsePhase.java:110)
at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:100)
... 7 more
This appears to be caused by the locking implementation in redirectTo():
public void redirectTo(String uri) {
warnIfSynchronous();
try {
view.acquireLifecycleLock();
view.installThreadLocals();
view.getFacesContext().getExternalContext().redirect(uri);
} catch (Exception e) {
throw new RuntimeException(e);
} finally {
release();
view.releaseLifecycleLock();
}
}
view.releaseLifecycleLock() releases all locks held by the current thread. Unfortunately, the current thread may be in the midst of processing a JSF lifecycle, making it vulnerable to corruption from other (push) threads acting on the same view.
Activity
| Field | Original Value | New Value |
|---|---|---|
| Salesforce Case | [] | |
| Fix Version/s | 1.8.2 [ 10190 ] | |
| Assignee | Ted Goddard [ ted.goddard ] |
Making use of the ExecutorService for redirectTo is likely less risky than modifying the locking strategy for View overall.
| Repository | Revision | Date | User | Message |
| ICEsoft Public SVN Repository | #19234 | Wed Sep 09 08:20:48 MDT 2009 | ted.goddard | running redirectTo via ExecutorService ( |
| Files Changed | ||||
MODIFY
/icefaces/trunk/icefaces/core/src/com/icesoft/faces/webapp/xmlhttp/PersistentFacesState.java
|
The following exception was observed with the fix in place. This exception has no functional impact, and simply indicates that the view being updated via push has been updated and a new view will be added to the render group in its place.
SEVERE: Missing ViewRoot in restoreState, ice.session: zYPxM_aPinG5rjfawptDZQ, viewNumber: 1
Sep 9, 2009 10:06:10 AM com.sun.faces.lifecycle.Phase doPhase
SEVERE: JSF1054: (Phase ID: RESTORE_VIEW 1, View ID: ) Exception thrown during phase execution: javax.faces.event.PhaseEvent[source=com.sun.faces.lifecycle.LifecycleImpl@211c7f8d]
Sep 9, 2009 10:06:10 AM com.icesoft.faces.webapp.xmlhttp.PersistentFacesState execute
SEVERE: Exception occured during execute push on Unknown View
javax.faces.application.ViewExpiredException: viewId:/auctionMonitor.iface - View /auctionMonitor.iface could not be restored.
at com.sun.faces.lifecycle.RestoreViewPhase.execute(RestoreViewPhase.java:189)
at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:100)
at com.sun.faces.lifecycle.RestoreViewPhase.doPhase(RestoreViewPhase.java:102)
at com.sun.faces.lifecycle.LifecycleImpl.execute(LifecycleImpl.java:118)
at com.icesoft.faces.webapp.xmlhttp.PersistentFacesState.execute(PersistentFacesState.java:285)
at com.icesoft.faces.webapp.xmlhttp.PersistentFacesState.executeAndRender(PersistentFacesState.java:311)
at com.icesoft.faces.async.render.RunnableRender.run(RunnableRender.java:143)
at edu.emory.mathcs.backport.java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:665)
at edu.emory.mathcs.backport.java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:690)
at java.lang.Thread.run(Thread.java:637)
Marking as fixed. Re-open or create new JIRA if redirectTo fails regression testing.
| Status | Open [ 1 ] | Resolved [ 5 ] |
| Resolution | Fixed [ 1 ] |
| Salesforce Case | [5007000000A6CuH] |
Why not add to the log for the exception above to indicate that this might be normal, or else catch the exception altogether?
This same exception can likely occur under abnormal circumstances as well, so it's best to leave it as-is. It occurs very rarely.
A test case is checked in, however, the test case does not make use of push, so it exercises redirectTo, but would not reproduce the exception.
| Fix Version/s | 1.8.2-RC1 [ 10210 ] |
| Status | Resolved [ 5 ] | Closed [ 6 ] |
— src/com/icesoft/faces/webapp/xmlhttp/PersistentFacesState.java (revision 19230)
+++ src/com/icesoft/faces/webapp/xmlhttp/PersistentFacesState.java (working copy)
@@ -331,18 +331,23 @@
*
*/
+ public void redirectTo(final String uri) {
warnIfSynchronous();
catch (Exception e)
{ - throw new RuntimeException(e); - }finally
{ - release(); - view.releaseLifecycleLock(); - }+ executorService.execute(new Runnable() {
+ public void run()
+ });
}
/**