Details
-
Type: Improvement
-
Status: Closed
-
Priority: Major
-
Resolution: Fixed
-
Affects Version/s: 1.8.2-EE-GA_P01
-
Fix Version/s: 1.8.3, 1.8.2-EE-GA_P02
-
Component/s: Framework
-
Labels:None
-
Environment:ICEfaces, Liferay, load test
Description
Activity
- All
- Comments
- History
- Activity
- Remote Attachments
- Subversion
— core/src/com/icesoft/faces/webapp/http/servlet/SessionDispatcher.java (revision 21691)
+++ core/src/com/icesoft/faces/webapp/http/servlet/SessionDispatcher.java (working copy)
@@ -44,6 +44,7 @@
import java.util.Collection;
import java.util.Date;
import java.util.HashMap;
+import java.util.WeakHashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@@ -67,7 +68,7 @@
//ICE-3073 - manage sessions with this structure
private final static Map SessionMonitors = new HashMap();
- private final Map sessionBoundServers = new HashMap();
+ private final Map sessionBoundServers = new WeakHashMap();
private final Map activeRequests = new HashMap();
private final String sessionIdDelimiter;
@@ -122,11 +123,15 @@
synchronized (sessionBoundServers) {
if (!sessionBoundServers.containsKey(id)) {
- sessionBoundServers.put(id, this.newServer(session, monitor, new Authorization() {
+ PseudoServlet pservlet = this.newServer(session, monitor, new Authorization() {
public boolean isUserInRole(String role) { return inRole(id, role); } - }));
+ });
+ //add to the session so that our WeakHashMap does not ignore the
+ //reference
+ session.setAttribute(id + ":sessionboundserver", pservlet);
+ sessionBoundServers.put(id, pservlet);
}
}
}
Index: core/src/com/icesoft/faces/webapp/http/core/SendUpdatedViews.java
===================================================================-
- core/src/com/icesoft/faces/webapp/http/core/SendUpdatedViews.java (revision 21691)
+++ core/src/com/icesoft/faces/webapp/http/core/SendUpdatedViews.java (working copy)
@@ -44,6 +44,7 @@
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
+import java.lang.ref.WeakReference;
- core/src/com/icesoft/faces/webapp/http/core/SendUpdatedViews.java (revision 21691)
-
public class SendUpdatedViews implements Server, Runnable {
private static final Runnable NOOP = new Runnable() {
@@ -69,6 +70,7 @@
private final MonitorRunner monitorRunner;
private long responseTimeoutTime;
private Server activeServer;
+ private WeakReference pageTestReference;
public SendUpdatedViews(String sessionID, final Collection synchronouslyUpdatedViews, final ViewQueue allUpdatedViews, final MonitorRunner monitorRunner, Configuration configuration, final PageTest pageTest) {
this.timeoutInterval = configuration.getAttributeAsLong("blockingConnectionTimeout", 90000);
@@ -76,6 +78,7 @@
this.allUpdatedViews = allUpdatedViews;
this.synchronouslyUpdatedViews = synchronouslyUpdatedViews;
this.monitorRunner = monitorRunner;
+ this.pageTestReference = new WeakReference(pageTest);
allUpdatedViews.onPut(new Runnable() {
public void run() {
@@ -107,9 +110,12 @@
//after first request switch to blocking server
activeServer = blockingServer;
+ //will throw NullPointerException if the session expires
+ //and this service method is somehow invoked
+ PageTest pageTester = (PageTest) pageTestReference.get();
//ICE-3493 – test if there was a previous request made for loading the page,
//if not assume that this request was meant for a failed cluster node
- if (pageTest.isLoaded()) {
+ if (pageTester.isLoaded()) { //page was loaded from this node, so use the blocking server super.service(request); }else {
Another fix may be to modify MonitorRunner so that the Runnable instances are removed from the ArrayList prior to run() so that the shutdown() is not necessary for this cleanup.
public MonitorRunner(final long interval) {
try {
Thread thread = new Thread("Monitor Runner") {
public void run() {
while (run) {
try {
Thread.sleep(interval);
Iterator i = new ArrayList(monitors).iterator();
while (i.hasNext()) {
Runnable monitor = (Runnable) i.next();
try
catch (Throwable t)
{ log.warn("Failed to run monitor: " + monitor); } }
} catch (InterruptedException e)
}
}
};
thread.setDaemon(true);
thread.start();
} catch (Exception e)
}
Entries are apparently removed from the map via:
private void sessionDestroy(final String sessionId)
{ sessionBoundServers.remove(sessionId); }However, if the sessionId is slightly modified or sessionDestroy() is not called as expected, the map entries will remain.
ASCII-art from the Eclipse memory analyzer (use monospaced font and a wide window to view):
A large number of SendUpdatedViews are referenced in the monitors array:
Class Name | Ref. Objects | Shallow Heap | Ref. Shallow Heap | Retained Heap
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
com.icesoft.util.MonitorRunner$1 @ 0xfffffffe6cd92500 Monitor Runner Thread | 2,561 | 184 | 430,248 | 3,720
'- this$0 com.icesoft.util.MonitorRunner @ 0xfffffffe6cd18f40 | 2,561 | 32 | 430,248 | 9,672
'- monitors java.util.ArrayList @ 0xfffffffe6cd18f60 | 2,561 | 40 | 430,248 | 9,640
'- elementData java.lang.Object[679] @ 0xfffffffe91963ae0 | 2,561 | 5,456 | 430,248 | 9,600
|
51 | 80 | 8,568 | 272 |
|
27 | 80 | 4,536 | 272 |
'- activeServer com.icesoft.faces.webapp.http.core.SendUpdatedViews$5 @ 0xfffffffefd6f3fe8 | 27 | 48 | 4,536 | 72 |
'- val$pageTest com.icesoft.faces.webapp.http.servlet.MainSessionBoundServlet @ 0xfffffffefd6f31d0 | 27 | 88 | 4,536 | 2,645,568 |
'- views java.util.Collections$SynchronizedMap @ 0xfffffffefd6f33a0 | 27 | 56 | 4,536 | 838,808 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
views held by MainServlet pageTest:
Class Name | Ref. Objects | Shallow Heap | Ref. Shallow Heap | Retained Heap
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
[2] class com.icesoft.faces.util.event.servlet.ContextEventRepeater @ 0xfffffffe3d832058 | 1,438 | 96 | 241,584 | 239,584
'- servletContextConfiguration com.icesoft.faces.webapp.http.servlet.ServletContextConfiguration @ 0xfffffffe6d980a30 | 1,438 | 32 | 241,584 | 32
'- context weblogic.servlet.internal.WebAppServletContext @ 0xfffffffe614ce790 | 1,438 | 392 | 241,584 | 565,480
'- attributes weblogic.servlet.internal.AttributesMap @ 0xfffffffe61789098 | 1,438 | 32 | 241,584 | 4,816
'- attributes java.util.concurrent.ConcurrentHashMap @ 0xfffffffe617890b8 | 1,438 | 72 | 241,584 | 4,784
'- segments java.util.concurrent.ConcurrentHashMap$Segment[16] @ 0xfffffffe61789100 | 1,438 | 152 | 241,584 | 4,712
'- [4] java.util.concurrent.ConcurrentHashMap$Segment @ 0xfffffffe617898f0 | 1,438 | 48 | 241,584 | 1,160
'- table java.util.concurrent.ConcurrentHashMap$HashEntry[2] @ 0xfffffffe6d73d008 | 1,438 | 40 | 241,584 | 1,064
'- [1] java.util.concurrent.ConcurrentHashMap$HashEntry @ 0xfffffffe6d73d030 | 1,438 | 48 | 241,584 | 48
'- value com.icesoft.faces.webapp.http.servlet.MainServlet$3 @ 0xfffffffe6cd18c28 | 1,438 | 80 | 241,584 | 219,182,600
'- sessionBoundServers java.util.HashMap @ 0xfffffffe6cd18c78 | 1,438 | 64 | 241,584 | 219,180,256
'- table java.util.HashMap$Entry[1024] @ 0xfffffffe7a21b410 | 1,438 | 8,216 | 241,584 | 219,180,192
|
58 | 48 | 9,744 | 11,769,192 | |
|
51 | 48 | 8,568 | 10,412,960 | |
|
7 | 88 | 1,176 | 1,356,184 | |
'- views java.util.Collections$SynchronizedMap @ 0xffffffff019d0548 | 7 | 56 | 1,176 | 632 ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
We have client branches that have most of these issues patched. We first need to audit the patches, add any that aren't already on the trunk, and re-test.
Waiting on audit and analysis of relevant customer patches with include memory fixes for portlets.
As a result of the work done in ICE-6197 to merge the changes from two customer patch branches, the various fixes for these issues have now been migrated back to the trunk.
SessionDispatcher is a singleton attached to MainServlet.
It had a direct reference to a map of sessionBoundServers which belonged to a variety of sessions.
This has been replaced with a WeakHashMap, so if the session goes away, we do not keep the reference.
A key was also added to the session itself, because without that the WeakHashMap would not keep the objects and they would likely go away immediately.
SendUpdatedViews was modified so that globally accumulated updates would not hold a reference to the MainSessionBoundServlet beyond the session lifetime.