[
http://jira.nuxeo.org/browse/NXP-3017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=48246#action_48246
]
Olivier Grisel commented on NXP-3017:
-------------------------------------
Proposed patch against the 5.1 branch:
diff --git
a/nuxeo-platform-directory-facade/src/main/java/org/nuxeo/ecm/platform/directory/ejb/DirectoryManagerBean.java
b/nuxeo-platform-directory-facade/src/main/java/org/nuxeo/ecm/platform/directory/ejb/DirectoryManagerBean.java
---
a/nuxeo-platform-directory-facade/src/main/java/org/nuxeo/ecm/platform/directory/ejb/DirectoryManagerBean.java
+++
b/nuxeo-platform-directory-facade/src/main/java/org/nuxeo/ecm/platform/directory/ejb/DirectoryManagerBean.java
@@ -1,5 +1,5 @@
/*
- * (C) Copyright 2006-2007 Nuxeo SAS (http://nuxeo.com/) and contributors.
+ * (C) Copyright 2006-2009 Nuxeo SAS (http://nuxeo.com/) and contributors.
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the GNU Lesser General Public License
@@ -31,6 +31,7 @@
import javax.ejb.PrePassivate;
import javax.ejb.Remote;
import javax.ejb.Stateful;
+import javax.ejb.Remove;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -48,33 +49,31 @@
/**
* @author <a href="mailto:[email protected]";>George Lefter</a>
- *
+ * @author <a href="mailto:[email protected]";>Olivier Grisel</a>
*/
@Stateful
-// XXX OG+GR: this bean is definitely not thread-safe.
-// think of it while removing the jboss specific annotation
@SerializedConcurrentAccess
@Remote(DirectoryManager.class)
@Local(DirectoryManager.class)
public class DirectoryManagerBean implements DirectoryManager {
- @SuppressWarnings("unused")
+
private static final Log log =
LogFactory.getLog(DirectoryManagerBean.class);
- private static final Map<Long, Session> sessionMap = new HashMap<Long,
Session>();
+ // do not forget to make the following maps thread-safe with
+ // Collections.synchronizedMap if the @SerializedConcurrentAccess
annotation
+ // is ever removed
- private static final Map<Long, String> sessionDirectoryNames = new
HashMap<Long, String>();
+ private final Map<Long, Session> sessionMap = new HashMap<Long,
Session>();
+
+ private final Map<Long, String> sessionDirectoryNames = new HashMap<Long,
String>();
+
+ private final AtomicLong sessionIdCounter = new AtomicLong(0);
private transient DirectoryService directoryService;
- private AtomicLong sessionIdCounter = new AtomicLong(0);
-
@PostActivate
@PostConstruct
public void initialize() {
- // UserService userService = (UserService) Framework.getRuntime()
- // .getComponent(UserService.NAME);
- // userManager = userService.getUserManager();
- // sessionMap = new HashMap<Long, Session>();
getService();
}
@@ -82,21 +81,31 @@
if (directoryService == null) {
directoryService =
Framework.getLocalService(DirectoryService.class);
}
-
return directoryService;
}
@PrePassivate
- public void cleanup() {
- directoryService = null;
+ @Remove
+ public void cleanupOpenSessions() {
+ // close any open directory sessions upon passivation or removal
+ // the sessionMap should be empty anyway since the client code should
+ // only use short lived directory clients in try / finally blocks
+ if (!sessionMap.isEmpty()) {
+ log.warn(String.format(
+ "about to close directory %d directory sessions:"
+ + " client code should not keep open directory
session",
+ sessionMap.size()));
+ }
for (Session session : sessionMap.values()) {
try {
session.close();
} catch (DirectoryException e) {
- // ignore
+ log.error("failed to close directory session properly: "
+ + e.getMessage(), e);
}
}
sessionMap.clear();
+ sessionDirectoryNames.clear();
}
private Session getSession(long sessionId) throws DirectoryException {
@@ -108,7 +117,7 @@
"Could not find directory name while rebuilding
session with id: "
+ sessionId);
}
- session = directoryService.open(directoryName);
+ session = getService().open(directoryName);
}
return session;
}
@@ -330,26 +339,26 @@
public String getDirectoryIdField(String directoryName)
throws DirectoryException {
- return directoryService.getDirectoryIdField(directoryName);
+ return getService().getDirectoryIdField(directoryName);
}
public String getDirectoryPasswordField(String directoryName)
throws DirectoryException {
- return directoryService.getDirectoryPasswordField(directoryName);
+ return getService().getDirectoryPasswordField(directoryName);
}
public void registerDirectory(String directoryName, DirectoryFactory
factory) {
- directoryService.registerDirectory(directoryName, factory);
+ getService().registerDirectory(directoryName, factory);
}
public void unregisterDirectory(String directoryName,
DirectoryFactory factory) {
- directoryService.unregisterDirectory(directoryName, factory);
+ getService().unregisterDirectory(directoryName, factory);
}
public String getParentDirectoryName(String directoryName)
throws DirectoryException {
- return directoryService.getParentDirectoryName(directoryName);
+ return getService().getParentDirectoryName(directoryName);
}
}
> Fix DirectoryManagerBean bad concurrency management when closing sessions
> -------------------------------------------------------------------------
>
> Key: NXP-3017
> URL: http://jira.nuxeo.org/browse/NXP-3017
> Project: Nuxeo Enterprise Platform
> Issue Type: Bug
> Components: Directory
> Affects Versions: 5.1.6, 5.2 M3
> Reporter: Olivier Grisel
> Assignee: Olivier Grisel
> Priority: Major
> Fix For: 5.1.7, 5.2 M4
>
>
> When using pages with heavy directory usage (such as forms with many
> vocabulary access) with several concurrent users one can get the following
> crash:
> Caused by: org.nuxeo.ecm.directory.DirectoryException: Could not find
> directory name while rebuilding session with id: 1
> at
> org.nuxeo.ecm.platform.directory.ejb.DirectoryManagerBean.getSession(DirectoryManagerBean.java:107)
> at
> org.nuxeo.ecm.platform.directory.ejb.DirectoryManagerBean.close(DirectoryManagerBean.java:127)
> at sun.reflect.GeneratedMethodAccessor150.invoke(Unknown Source)
> This is due to the fact that the sessions maps are static while the session
> counter is not. A proposed patch will be attached in comments.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://jira.nuxeo.org/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
_______________________________________________
ECM-tickets mailing list
[email protected]
http://lists.nuxeo.com/mailman/listinfo/ecm-tickets
