Skip to content

Commit

Permalink
EclipseContext: allow GC for undisposed contexts #375
Browse files Browse the repository at this point in the history 
* WeakGroupedListenerList:
Use JDK's WeakHashMap (with a cleaner) to not leak
WeakComputationReference

* InjectorImpl:
Use WeakHashMap to avoid strong references to injected Contexts to avoid
leaked EclipseContext in the key of InjectorImpl.injectedObjects

* Fix modifiers and their order

Fixes Eclipse-IDE process stuck in EclipseContext.cleanup during
shutdown. Happened if EclipseContext could be garbage collected but is
not explicitly disposed.
  • Loading branch information
@EcljpseB0T @jukzi
EcljpseB0T authored and jukzi committed Mar 31, 2023
1 parent 3d6a492 commit 488483c
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 132 deletions.
132 changes: 39 additions & 93 deletions ...e.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/WeakGroupedListenerList.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2012, 2015 IBM Corporation and others.
* Copyright (c) 2012, 2023 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -14,136 +14,82 @@

package org.eclipse.e4.core.internal.contexts;

import java.lang.ref.WeakReference;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
import java.util.WeakHashMap;

/**
* Listeners are held wrapped in weak references and are removed if no other [strong] reference
* exists.
*/
public class WeakGroupedListenerList {

public static class WeakComputationReference extends WeakReference<Computation> {
private final Map<String, Set<Computation>> listeners = new HashMap<>();

final private int hashCode;

public WeakComputationReference(Computation computation) {
super(computation);
hashCode = computation.hashCode();
}

@Override
public int hashCode() {
return hashCode;
}

@Override
public boolean equals(Object obj) {
if (obj == null)
return false;
if (!WeakComputationReference.class.equals(obj.getClass()))
return super.equals(obj);
Computation computation = get();
Computation otherComputation = ((WeakComputationReference) obj).get();
if (computation == null && otherComputation == null)
return true;
if (computation == null || otherComputation == null)
return false;
return computation.equals(otherComputation);
}
}

private Map<String, HashSet<WeakComputationReference>> listeners = new HashMap<>(10, 0.8f);

synchronized public void add(String groupName, Computation computation) {
HashSet<WeakComputationReference> nameListeners = listeners.get(groupName);
if (nameListeners == null) {
nameListeners = new HashSet<>(30, 0.75f);
nameListeners.add(new WeakComputationReference(computation));
listeners.put(groupName, nameListeners);
}
nameListeners.add(new WeakComputationReference(computation));
public synchronized void add(String groupName, Computation computation) {
Objects.requireNonNull(computation);
listeners.computeIfAbsent(groupName,
k -> Collections.newSetFromMap(new WeakHashMap<Computation, Boolean>()))
.add(computation);
}

synchronized public void remove(Computation computation) {
WeakComputationReference ref = new WeakComputationReference(computation);
Collection<HashSet<WeakComputationReference>> allListeners = listeners.values();
for (HashSet<WeakComputationReference> group : allListeners) {
group.remove(ref);
public synchronized void remove(Computation computation) {
Collection<Set<Computation>> allListeners = listeners.values();
for (Set<Computation> group : allListeners) {
group.remove(computation);
}
}

synchronized public Set<String> getNames() {
Set<String> tmp = listeners.keySet(); // clone internal name list
Set<String> usedNames = new HashSet<>(tmp.size());
usedNames.addAll(tmp);
public synchronized Set<String> getNames() {
Set<String> groupNames = listeners.keySet(); // clone internal name list
Set<String> usedNames = new HashSet<>(groupNames.size());
usedNames.addAll(groupNames);
return usedNames;
}

synchronized public void clear() {
public synchronized void clear() {
listeners.clear();
}

synchronized public Set<Computation> getListeners() {
Collection<HashSet<WeakComputationReference>> collection = listeners.values();
public synchronized Set<Computation> getListeners() {
Collection<Set<Computation>> groups = listeners.values();
Set<Computation> result = new HashSet<>();
for (HashSet<WeakComputationReference> set : collection) {
for (Iterator<WeakComputationReference> i = set.iterator(); i.hasNext();) {
WeakComputationReference ref = i.next();
Computation computation = ref.get();
if (computation == null || !computation.isValid()) {
i.remove(); // do a clean-up while we are here
} else
for (Set<Computation> computations : groups) {
for (Computation computation : computations) {
if (computation.isValid()) {
result.add(computation);
}
}
}
return result;
}

synchronized public Set<Computation> getListeners(String groupName) {
HashSet<WeakComputationReference> tmp = listeners.get(groupName);
if (tmp == null)
public synchronized Set<Computation> getListeners(String groupName) {
Set<Computation> computations = listeners.get(groupName);
if (computations == null)
return null;
Set<Computation> result = new HashSet<>(tmp.size());

for (Iterator<WeakComputationReference> i = tmp.iterator(); i.hasNext();) {
WeakComputationReference ref = i.next();
Computation computation = ref.get();
if (computation == null || !computation.isValid()) {
i.remove(); // do a clean-up while we are here
} else
Set<Computation> result = new HashSet<>(computations.size());
for (Computation computation : computations) {
if (computation.isValid()) {
result.add(computation);
}
}
return result;
}

synchronized public void cleanup() {
boolean cleanGroups = false;
for (HashSet<WeakComputationReference> set : listeners.values()) {
for (Iterator<WeakComputationReference> i = set.iterator(); i.hasNext();) {
WeakComputationReference ref = i.next();
Computation computation = ref.get();
if (computation == null || !computation.isValid())
i.remove();
}
if (set.isEmpty())
cleanGroups = true;
}
if (cleanGroups) {
Set<Entry<String, HashSet<WeakComputationReference>>> entries = listeners.entrySet();
for (Iterator<Entry<String, HashSet<WeakComputationReference>>> i = entries.iterator(); i.hasNext();) {
Entry<String, HashSet<WeakComputationReference>> entry = i.next();
HashSet<WeakComputationReference> value = entry.getValue();
if (value == null || value.isEmpty())
i.remove();
}
}
public synchronized void cleanup() {
Set<Entry<String, Set<Computation>>> entries = listeners.entrySet();
entries.removeIf(entry -> {
Set<Computation> computations = entry.getValue();
computations.removeIf(computation -> !computation.isValid());
return computations.isEmpty();
});
}

}
70 changes: 31 additions & 39 deletions runtime/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/InjectorImpl.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2009, 2017 IBM Corporation and others.
* Copyright (c) 2009, 2023 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -61,34 +61,35 @@
*/
public class InjectorImpl implements IInjector {

final static private boolean shouldDebug = Boolean.getBoolean("org.eclipse.e4.core.di.debug"); //$NON-NLS-1$
private static final boolean shouldDebug = Boolean.getBoolean("org.eclipse.e4.core.di.debug"); //$NON-NLS-1$

final static private String JAVA_OBJECT = "java.lang.Object"; //$NON-NLS-1$
private static final String JAVA_OBJECT = "java.lang.Object"; //$NON-NLS-1$

final private static Boolean DEFAULT_BOOLEAN = Boolean.FALSE;
final private static Integer DEFAULT_INTEGER = Integer.valueOf(0);
final private static Character DEFAULT_CHAR = Character.valueOf((char) 0);
final private static Float DEFAULT_FLOAT = Float.valueOf(0.0f);
final private static Double DEFAULT_DOUBLE = Double.valueOf(0.0d);
final private static Long DEFAULT_LONG = Long.valueOf(0L);
final private static Short DEFAULT_SHORT = Short.valueOf((short) 0);
final private static Byte DEFAULT_BYTE = Byte.valueOf((byte) 0);
private final static Boolean DEFAULT_BOOLEAN = Boolean.FALSE;
private final static Integer DEFAULT_INTEGER = Integer.valueOf(0);
private final static Character DEFAULT_CHAR = Character.valueOf((char) 0);
private final static Float DEFAULT_FLOAT = Float.valueOf(0.0f);
private final static Double DEFAULT_DOUBLE = Double.valueOf(0.0d);
private final static Long DEFAULT_LONG = Long.valueOf(0L);
private final static Short DEFAULT_SHORT = Short.valueOf((short) 0);
private final static Byte DEFAULT_BYTE = Byte.valueOf((byte) 0);

private Map<PrimaryObjectSupplier, List<WeakReference<?>>> injectedObjects = new HashMap<>();
private Set<WeakReference<Class<?>>> injectedClasses = new HashSet<>();
private HashMap<Class<?>, Object> singletonCache = new HashMap<>();
private Map<Class<?>, Set<Binding>> bindings = new HashMap<>();
private Map<Class<? extends Annotation>, Map<AnnotatedElement, Boolean>> annotationsPresent = new HashMap<>();
private final Map<PrimaryObjectSupplier, List<WeakReference<?>>> injectedObjects = new WeakHashMap<>();
private final Set<WeakReference<Class<?>>> injectedClasses = new HashSet<>();
private final HashMap<Class<?>, Object> singletonCache = new HashMap<>();
private final Map<Class<?>, Set<Binding>> bindings = new HashMap<>();
private final Map<Class<? extends Annotation>, Map<AnnotatedElement, Boolean>> annotationsPresent = new HashMap<>();

// Performance improvement:
private Map<Class<?>, Method[]> methodsCache = Collections.synchronizedMap(new WeakHashMap<>());
private Map<Class<?>, Field[]> fieldsCache = Collections.synchronizedMap(new WeakHashMap<>());
private Map<Class<?>, Constructor<?>[]> constructorsCache = Collections.synchronizedMap(new WeakHashMap<>());
private Map<Class<?>, Map<Method, Boolean>> isOverriddenCache = Collections.synchronizedMap(new WeakHashMap<>());
private final Map<Class<?>, Method[]> methodsCache = Collections.synchronizedMap(new WeakHashMap<>());
private final Map<Class<?>, Field[]> fieldsCache = Collections.synchronizedMap(new WeakHashMap<>());
private final Map<Class<?>, Constructor<?>[]> constructorsCache = Collections.synchronizedMap(new WeakHashMap<>());
private final Map<Class<?>, Map<Method, Boolean>> isOverriddenCache = Collections
.synchronizedMap(new WeakHashMap<>());

private Set<Class<?>> classesBeingCreated = new HashSet<>(5);
private final Set<Class<?>> classesBeingCreated = new HashSet<>(5);

private PrimaryObjectSupplier defaultSupplier;
private volatile PrimaryObjectSupplier defaultSupplier;

@Override
public void inject(Object object, PrimaryObjectSupplier objectSupplier) {
Expand Down Expand Up @@ -147,12 +148,7 @@ private void internalInject(Object object, PrimaryObjectSupplier objectSupplier,

private void rememberInjectedObject(Object object, PrimaryObjectSupplier objectSupplier) {
synchronized (injectedObjects) {
List<WeakReference<?>> list;
if (!injectedObjects.containsKey(objectSupplier)) {
list = new ArrayList<>();
injectedObjects.put(objectSupplier, list);
} else
list = injectedObjects.get(objectSupplier);
List<WeakReference<?>> list = injectedObjects.computeIfAbsent(objectSupplier, k -> new ArrayList<>());
for (WeakReference<?> ref : list) {
if (object == ref.get())
return; // we already have it
Expand All @@ -163,14 +159,14 @@ private void rememberInjectedObject(Object object, PrimaryObjectSupplier objectS

private boolean forgetInjectedObject(Object object, PrimaryObjectSupplier objectSupplier) {
synchronized (injectedObjects) {
if (!injectedObjects.containsKey(objectSupplier))
return false;
List<WeakReference<?>> list = injectedObjects.get(objectSupplier);
for (Iterator<WeakReference<?>> i = list.iterator(); i.hasNext();) {
WeakReference<?> ref = i.next();
if (object == ref.get()) {
i.remove();
return true;
if (list != null) {
for (Iterator<WeakReference<?>> i = list.iterator(); i.hasNext();) {
WeakReference<?> ref = i.next();
if (object == ref.get()) {
i.remove();
return true;
}
}
}
return false;
Expand All @@ -179,16 +175,12 @@ private boolean forgetInjectedObject(Object object, PrimaryObjectSupplier object

private List<WeakReference<?>> forgetSupplier(PrimaryObjectSupplier objectSupplier) {
synchronized (injectedObjects) {
if (!injectedObjects.containsKey(objectSupplier))
return null;
return injectedObjects.remove(objectSupplier);
}
}

private List<WeakReference<?>> getSupplierObjects(PrimaryObjectSupplier objectSupplier) {
synchronized (injectedObjects) {
if (!injectedObjects.containsKey(objectSupplier))
return null;
return injectedObjects.get(objectSupplier);
}
}
Expand Down

0 comments on commit 488483c

Please sign in to comment.