eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
159 stars 109 forks source link

ViewScopeContextManager: incorrect synchronization #3743

Closed ren-zhijun-oracle closed 9 years ago

ren-zhijun-oracle commented 9 years ago

com.sun.faces.application.view.ViewScopeContextManager.getContextMap(FacesContext, boolean):

Map<String, Object> sessionMap = externalContext.getSessionMap();
Map<Object, Map<String, ViewScopeContextObject>> activeViewScopeContexts =
        (Map<Object, Map<String, ViewScopeContextObject>>) sessionMap.get(ACTIVE_VIEW_CONTEXTS);
Map<String, Object> viewMap = facesContext.getViewRoot().getViewMap(false);

if (activeViewScopeContexts == null && create) {
    synchronized (sessionMap) {
        activeViewScopeContexts = new ConcurrentHashMap<Object, Map<String, ViewScopeContextObject>>();
        sessionMap.put(ACTIVE_VIEW_CONTEXTS, activeViewScopeContexts);
    }
}

There are 2 problems here:

  1. The sessionMap is a field of ExternalContextImpl, which is effectively request scoped, so synchronizing on the sessionMap is useless. Probably should use some lock stored in session attributes.
  2. The synchronizing and checking block is incorrect. It should be either a straightforward:``` synchronized (lock) { if (activeViewScopeContexts == null && create) { activeViewScopeContexts = new ConcurrentHashMap<Object, Map<String, ViewScopeContextObject>>(); sessionMap.put(ACTIVE_VIEW_CONTEXTS, activeViewScopeContexts); } }

    
    or a double checked:
    

    if (activeViewScopeContexts == null && create) { synchronized (lock) { activeViewScopeContexts = sessionMap.get(ACTIVE_VIEW_CONTEXTS); if (activeViewScopeContexts == null) { activeViewScopeContexts = new ConcurrentHashMap<Object, Map<String, ViewScopeContextObject>>(); sessionMap.put(ACTIVE_VIEW_CONTEXTS, activeViewScopeContexts); } } }

    
    #### Environment
    Wildfly 8.1.0.Final (Mojarra updated to 2.2.8).
    #### Affected Versions
    [2.2.8]
ren-zhijun-oracle commented 6 years ago
ren-zhijun-oracle commented 9 years ago

@javaserverfaces Commented Reported by VsevolodGolovanov

ren-zhijun-oracle commented 9 years ago

@javaserverfaces Commented @manfredriem said: Applied to 2.3 trunk,

svn commit -m "Fixes https://java.net/jira/browse/JAVASERVERFACES-3739, ViewScopeContextManager: incorrect synchronization" Sending jsf-ri/src/main/java/com/sun/faces/application/view/ViewScopeContextManager.java Transmitting file data . Committed revision 14433.

ren-zhijun-oracle commented 9 years ago

@javaserverfaces Commented File: changebundle.txt Attached By: @manfredriem

ren-zhijun-oracle commented 7 years ago

@javaserverfaces Commented This issue was imported from java.net JIRA JAVASERVERFACES-3739

ren-zhijun-oracle commented 9 years ago

@javaserverfaces Commented Marked as fixed on Tuesday, February 24th 2015, 8:42:39 am

VsevolodGolovanov commented 7 months ago

1.

if (activeViewScopeContexts == null && create) {
    synchronized (session) {
        activeViewScopeContexts = new ConcurrentHashMap<>();
        sessionMap.put(ACTIVE_VIEW_CONTEXTS, activeViewScopeContexts);
    }
}

It synchronizes on the session object. As mentioned before in https://github.com/eclipse-ee4j/mojarra/issues/3738 the session object is a different instance in each request at least with Undertow. See also https://github.com/jakartaee/servlet/issues/122.

(Just leaving a note here, because it's not important enough to be properly reported, and https://github.com/eclipse-ee4j/mojarra/issues/3738 was dismissed anyway.)

BalusC commented 7 months ago

It warrants a fix. Please create a new issue.