From 50c59ad9f307d16bc312d9b87b9fcfda1ea8f44c Mon Sep 17 00:00:00 2001 From: sb Date: Tue, 24 Nov 2009 11:44:08 +0100 Subject: sb116: #i107157# redesigned java_environment.Registry to avoid memory leaks (and simplified code) --- .../uno/environments/java/java_environment.java | 192 ++++++++------------- 1 file changed, 76 insertions(+), 116 deletions(-) diff --git a/jurt/com/sun/star/lib/uno/environments/java/java_environment.java b/jurt/com/sun/star/lib/uno/environments/java/java_environment.java index 36404f28d57e..aa9a21a26b22 100644 --- a/jurt/com/sun/star/lib/uno/environments/java/java_environment.java +++ b/jurt/com/sun/star/lib/uno/environments/java/java_environment.java @@ -37,7 +37,6 @@ import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; import java.util.HashMap; import java.util.Iterator; -import java.util.LinkedList; /** * The java_environment is the environment where objects and @@ -159,70 +158,64 @@ public final class java_environment implements IEnvironment { } private static final class Registry { - public Object register(Object object, String oid, Type type) { - synchronized (map) { - cleanUp(); - Level1Entry l1 = getLevel1Entry(oid); - if (l1 != null) { - Level2Entry l2 = l1.get(type); - if (l2 != null) { - Object o = l2.get(); - if (o != null) { - l2.acquire(); - return o; - } + public synchronized Object register( + Object object, String oid, Type type) + { + cleanUp(); + Level1Entry l1 = level1map.get(oid); + if (l1 != null) { + Level2Entry l2 = l1.level2map.get(type); + if (l2 != null) { + Object o = l2.get(); + if (o != null) { + l2.acquire(); + return o; } } - // TODO If a holder references an unreachable object, but still - // has a positive count, it is replaced with a new holder - // (referencing a reachable object, and with a count of 1). Any - // later calls to revoke that should decrement the count of the - // previous holder would now decrement the count of the new - // holder, removing it prematurely. This is a design flaw that - // will be fixed when IEnvironment.revokeInterface is changed to - // no longer use counting. (And this problem is harmless, as - // currently a holder either references a strongly held object - // and uses register/revoke to control it, or references a - // weakly held proxy and never revokes it.) - if (l1 == null) { - l1 = new Level1Entry(); - map.put(oid, l1); - } - l1.add(new Level2Entry(oid, type, object, queue)); } + // TODO If a holder references an unreachable object, but still has + // a positive count, it is replaced with a new holder (referencing a + // reachable object, and with a count of 1). Any later calls to + // revoke that should decrement the count of the previous holder + // would now decrement the count of the new holder, removing it + // prematurely. This is a design flaw that will be fixed when + // IEnvironment.revokeInterface is changed to no longer use + // counting. (And this problem is harmless, as currently a holder + // either references a strongly held object and uses register/revoke + // to control it, or references a weakly held proxy and never + // revokes it.) + if (l1 == null) { + l1 = new Level1Entry(); + level1map.put(oid, l1); + } + l1.level2map.put(type, new Level2Entry(oid, type, object, queue)); return object; } - public boolean revoke(String oid, Type type) { - synchronized (map) { - Level1Entry l1 = getLevel1Entry(oid); - Level2Entry l2 = null; - if (l1 != null) { - l2 = l1.get(type); - if (l2 != null && l2.release()) { - removeLevel2Entry(oid, l1, l2); - } + public synchronized boolean revoke(String oid, Type type) { + Level1Entry l1 = level1map.get(oid); + Level2Entry l2 = null; + if (l1 != null) { + l2 = l1.level2map.get(type); + if (l2 != null && l2.release()) { + removeLevel2Entry(l1, oid, type); } - cleanUp(); - return l2 != null; } + cleanUp(); + return l2 != null; } - public Object get(String oid, Type type) { - synchronized (map) { - Level1Entry l1 = getLevel1Entry(oid); - return l1 == null ? null : l1.find(type); - } + public synchronized Object get(String oid, Type type) { + Level1Entry l1 = level1map.get(oid); + return l1 == null ? null : l1.find(type); } - public void clear() { - synchronized (map) { - map.clear(); - cleanUp(); - } + public synchronized void clear() { + level1map.clear(); + cleanUp(); } - // must only be called while synchronized on map: + // must only be called while synchronized on this Registry: private void cleanUp() { for (;;) { Level2Entry l2 = (Level2Entry) queue.poll(); @@ -235,55 +228,38 @@ public final class java_environment implements IEnvironment { // created since now e1.get() == null), and only then e1 is // enqueued. To not erroneously remove the new e2 in that case, // check whether the map still contains e1: - String oid = l2.getOid(); - Level1Entry l1 = getLevel1Entry(oid); - if (l1 != null && l1.get(l2.getType()) == l2) { - removeLevel2Entry(oid, l1, l2); + Level1Entry l1 = level1map.get(l2.oid); + if (l1 != null && l1.level2map.get(l2.type) == l2) { + removeLevel2Entry(l1, l2.oid, l2.type); } } } - // must only be called while synchronized on map: - private Level1Entry getLevel1Entry(String oid) { - return (Level1Entry) map.get(oid); - } - - // must only be called while synchronized on map: - private void removeLevel2Entry(String oid, Level1Entry l1, - Level2Entry l2) { - if (l1.remove(l2)) { - map.remove(oid); + // must only be called while synchronized on this Registry: + private void removeLevel2Entry(Level1Entry l1, String oid, Type type) { + l1.level2map.remove(type); + if (l1.level2map.isEmpty()) { + level1map.remove(oid); } } private static final class Level1Entry { - // must only be called while synchronized on map: - public Level2Entry get(Type type) { - for (Iterator i = list.iterator(); i.hasNext();) { - Level2Entry l2 = (Level2Entry) i.next(); - if (l2.getType().equals(type)) { - return l2; - } - } - return null; - } - - // must only be called while synchronized on map: + // must only be called while synchronized on enclosing Registry: public Object find(Type type) { // First, look for an exactly matching entry; then, look for an // arbitrary entry for a subtype of the request type: - for (Iterator i = list.iterator(); i.hasNext();) { - Level2Entry l2 = (Level2Entry) i.next(); - if (l2.getType().equals(type)) { - Object o = l2.get(); - if (o != null) { - return o; - } + Level2Entry l2 = level2map.get(type); + if (l2 != null) { + Object o = l2.get(); + if (o != null) { + return o; } } - for (Iterator i = list.iterator(); i.hasNext();) { - Level2Entry l2 = (Level2Entry) i.next(); - if (type.isSupertypeOf(l2.getType())) { + for (Iterator i = level2map.values().iterator(); + i.hasNext();) + { + l2 = i.next(); + if (type.isSupertypeOf(l2.type)) { Object o = l2.get(); if (o != null) { return o; @@ -293,53 +269,37 @@ public final class java_environment implements IEnvironment { return null; } - // must only be called while synchronized on map: - public void add(Level2Entry l2) { - list.add(l2); - } - - // must only be called while synchronized on map: - public boolean remove(Level2Entry l2) { - list.remove(l2); - return list.isEmpty(); - } - - private final LinkedList list = new LinkedList(); // of Level2Entry + public final HashMap level2map = + new HashMap(); } - private static final class Level2Entry extends WeakReference { - public Level2Entry(String oid, Type type, Object object, - ReferenceQueue queue) { + private static final class Level2Entry extends WeakReference { + public Level2Entry( + String oid, Type type, Object object, ReferenceQueue queue) + { super(object, queue); this.oid = oid; this.type = type; } - public String getOid() { - return oid; - } - - public Type getType() { - return type; - } - - // must only be called while synchronized on map: + // must only be called while synchronized on enclosing Registry: public void acquire() { ++count; } - // must only be called while synchronized on map: + // must only be called while synchronized on enclosing Registry: public boolean release() { return --count == 0; } - private final String oid; - private final Type type; + public final String oid; + public final Type type; + private int count = 1; } - private final HashMap map = new HashMap(); - // from OID (String) to Level1Entry + private final HashMap level1map = + new HashMap(); private final ReferenceQueue queue = new ReferenceQueue(); } -- cgit