You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by pa...@apache.org on 2023/04/14 21:09:49 UTC
[felix-dev] branch master updated: FELIX-6591: Deallocate Conditions and Permissions when bundles unloaded. (#193)
This is an automated email from the ASF dual-hosted git repository.
pauls pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/felix-dev.git
The following commit(s) were added to refs/heads/master by this push:
new 9cadd32776 FELIX-6591: Deallocate Conditions and Permissions when bundles unloaded. (#193)
9cadd32776 is described below
commit 9cadd32776c9fa9ba845f124f09df8e50566b717
Author: Chris Rankin <ch...@r3.com>
AuthorDate: Fri Apr 14 22:09:43 2023 +0100
FELIX-6591: Deallocate Conditions and Permissions when bundles unloaded. (#193)
* Explicitly remove conditional permissions from uninstalled bundles.
* Fix management of Permissions and PermissionInfo objects.
---
.../apache/felix/framework/SecurityActivator.java | 36 ++++++++++++-
.../felix/framework/security/util/Conditions.java | 41 +++++++++++----
.../felix/framework/security/util/Permissions.java | 61 +++++++++++-----------
3 files changed, 98 insertions(+), 40 deletions(-)
diff --git a/framework.security/src/main/java/org/apache/felix/framework/SecurityActivator.java b/framework.security/src/main/java/org/apache/felix/framework/SecurityActivator.java
index 1106423cd1..fd762644bd 100644
--- a/framework.security/src/main/java/org/apache/felix/framework/SecurityActivator.java
+++ b/framework.security/src/main/java/org/apache/felix/framework/SecurityActivator.java
@@ -20,6 +20,8 @@ package org.apache.felix.framework;
import java.io.File;
import java.io.IOException;
+import java.util.Iterator;
+import java.util.List;
import java.util.StringTokenizer;
import org.apache.felix.framework.ext.SecurityProvider;
@@ -33,8 +35,11 @@ import org.apache.felix.framework.security.util.PropertiesCache;
import org.apache.felix.framework.util.SecureAction;
import org.osgi.framework.BundleActivator;
import org.osgi.framework.BundleContext;
+import org.osgi.framework.BundleEvent;
import org.osgi.framework.BundleException;
+import org.osgi.framework.BundleListener;
import org.osgi.framework.Constants;
+import org.osgi.framework.wiring.BundleRevisions;
import org.osgi.service.condpermadmin.ConditionalPermissionAdmin;
import org.osgi.service.permissionadmin.PermissionAdmin;
@@ -150,9 +155,12 @@ public final class SecurityActivator implements BundleActivator
LocalPermissions localPermissions = new LocalPermissions(
permissions);
+ final Conditions conditions = new Conditions(action);
cpai = new ConditionalPermissionAdminImpl(permissions,
- new Conditions(action), localPermissions, new PropertiesCache(
+ conditions, localPermissions, new PropertiesCache(
cpaCache, tmp, action), pai);
+
+ context.addBundleListener(new UninstallListener(conditions));
}
if ((pai != null) || (cpai != null))
@@ -231,4 +239,30 @@ public final class SecurityActivator implements BundleActivator
return (result != null) ? result : defaultValue;
}
+
+ private static final class UninstallListener implements BundleListener
+ {
+ private final Conditions conditions;
+
+ UninstallListener(Conditions conditions)
+ {
+ this.conditions = conditions;
+ }
+
+ public void bundleChanged(BundleEvent event)
+ {
+ if (event.getType() == BundleEvent.UNINSTALLED)
+ {
+ List revisions = ((BundleRevisions)event.getBundle().adapt(BundleRevisions.class)).getRevisions();
+ if (revisions != null)
+ {
+ Iterator iter = revisions.iterator();
+ while (iter.hasNext())
+ {
+ conditions.remove((BundleRevisionImpl) iter.next());
+ }
+ }
+ }
+ }
+ }
}
diff --git a/framework.security/src/main/java/org/apache/felix/framework/security/util/Conditions.java b/framework.security/src/main/java/org/apache/felix/framework/security/util/Conditions.java
index 34468ebdbd..739576cb24 100644
--- a/framework.security/src/main/java/org/apache/felix/framework/security/util/Conditions.java
+++ b/framework.security/src/main/java/org/apache/felix/framework/security/util/Conditions.java
@@ -39,11 +39,10 @@ import org.osgi.service.condpermadmin.ConditionInfo;
* This class caches conditions instances by their infos. Furthermore, it allows
* to eval postponed condition permission tuples as per spec (see 9.45).
*/
-// TODO: maybe use bundle events instead of soft/weak references.
public final class Conditions
{
private static final ThreadLocal m_conditionStack = new ThreadLocal();
- private static final Map m_conditionCache = new WeakHashMap();
+ private static final Map m_conditionCache = new HashMap();
private final Map m_cache = new WeakHashMap();
@@ -101,7 +100,7 @@ public final class Conditions
index = (Map) m_cache.get(conditions);
if (index == null)
{
- index = new WeakHashMap();
+ index = new HashMap();
m_cache.put(conditions, index);
}
}
@@ -111,13 +110,9 @@ public final class Conditions
{
result = (Conditions) index.get(key);
}
- }
-
- if (result == null)
- {
- result = new Conditions(key, conditions, m_action);
- synchronized (index)
+ if (result == null)
{
+ result = new Conditions(key, conditions, m_action);
index.put(key, result);
}
}
@@ -125,6 +120,34 @@ public final class Conditions
return result;
}
+ public void remove(BundleRevisionImpl key) {
+ final Map conditionMap;
+ synchronized (m_conditionCache)
+ {
+ conditionMap = (Map) m_conditionCache.remove(key);
+ }
+
+ if (conditionMap != null)
+ {
+ final Iterator iter = conditionMap.keySet().iterator();
+ if (iter.hasNext())
+ {
+ synchronized (m_cache)
+ {
+ do
+ {
+ final Map index = (Map) m_cache.get(iter.next());
+ if (index != null)
+ {
+ index.remove(key);
+ }
+ }
+ while (iter.hasNext());
+ }
+ }
+ }
+ }
+
// See whether the given list is satisfied or not
public boolean isSatisfied(List posts, Permissions permissions,
Permission permission)
diff --git a/framework.security/src/main/java/org/apache/felix/framework/security/util/Permissions.java b/framework.security/src/main/java/org/apache/felix/framework/security/util/Permissions.java
index 2e8ec42daa..df971594bd 100644
--- a/framework.security/src/main/java/org/apache/felix/framework/security/util/Permissions.java
+++ b/framework.security/src/main/java/org/apache/felix/framework/security/util/Permissions.java
@@ -50,7 +50,7 @@ import org.osgi.service.packageadmin.PackageAdmin;
import org.osgi.service.permissionadmin.PermissionInfo;
/**
- * A permission cache that uses permisssion infos as keys. Permission are
+ * A permission cache that uses permission infos as keys. Permission are
* created from the parent classloader or any exported package.
*/
// TODO: maybe use bundle events instead of soft/weak references
@@ -126,21 +126,18 @@ public final class Permissions
{
cleanUp(m_permissionsQueue, m_permissions);
- Permissions result = null;
+ Permissions result;
synchronized (m_permissions)
{
result = (Permissions) m_permissions.get(new Entry(permissionInfos));
- }
- if (result == null)
- {
- //permissionInfos may not be referenced by the new Permissions, as
- //otherwise the reference in m_permissions prevents the key from
- //being garbage collectable.
- PermissionInfo[] permissionInfosClone = new PermissionInfo[permissionInfos.length];
- System.arraycopy(permissionInfos, 0, permissionInfosClone, 0, permissionInfos.length);
- result = new Permissions(permissionInfosClone, m_context, m_action);
- synchronized (m_permissions)
+ if (result == null)
{
+ //permissionInfos may not be referenced by the new Permissions, as
+ //otherwise the reference in m_permissions prevents the key from
+ //being garbage collectable.
+ PermissionInfo[] permissionInfosClone = new PermissionInfo[permissionInfos.length];
+ System.arraycopy(permissionInfos, 0, permissionInfosClone, 0, permissionInfos.length);
+ result = new Permissions(permissionInfosClone, m_context, m_action);
m_permissions.put(
new Entry(permissionInfos, m_permissionsQueue), result);
}
@@ -152,21 +149,27 @@ public final class Permissions
{
private final int m_hashCode;
- Entry(Object entry, ReferenceQueue queue)
+ // Replace with Arrays.hashCode(Object[]) by supporting Java 5+.
+ private static int hashCode(Object[] array)
{
- super(entry, queue);
- m_hashCode = entry.hashCode();
+ int hash = 0;
+ for (int i = 0; i < array.length; ++i)
+ {
+ Object element = array[i];
+ hash = hash * 31 + ((element == null) ? 0 : element.hashCode());
+ }
+ return hash;
}
- Entry(Object entry)
+ Entry(Object entry, ReferenceQueue queue)
{
- super(entry);
- m_hashCode = entry.hashCode();
+ super(entry, queue);
+ m_hashCode = entry instanceof Object[] ? hashCode((Object[]) entry): entry.hashCode();
}
- public Object get()
+ Entry(Object entry)
{
- return super.get();
+ this(entry, null);
}
public int hashCode()
@@ -186,12 +189,12 @@ public final class Permissions
return true;
}
- Object entry = super.get();
+ final Object entry = get();
if (o instanceof Entry)
{
- Object otherEntry = ((Entry) o).get();
+ final Object otherEntry = ((Entry) o).get();
if (entry == null)
{
return otherEntry == null;
@@ -208,7 +211,7 @@ public final class Permissions
{
return Arrays.equals((Object[])entry, (Object[])otherEntry);
}
- return entry.equals(((Entry) o).get());
+ return entry.equals(otherEntry);
}
else
{
@@ -384,18 +387,16 @@ public final class Permissions
try
{
- SoftReference collectionEntry = null;
-
PermissionCollection collection = null;
synchronized (m_cache)
{
- collectionEntry = (SoftReference) m_cache.get(targetClass);
- }
+ final SoftReference collectionEntry = (SoftReference) m_cache.get(targetClass);
- if (collectionEntry != null)
- {
- collection = (PermissionCollection) collectionEntry.get();
+ if (collectionEntry != null)
+ {
+ collection = (PermissionCollection) collectionEntry.get();
+ }
}
if (collection == null)