From a74bbe6ad8d381c7b28f46cdd2c5ff52c6bc247e Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Fri, 15 Nov 2024 13:52:22 -0600 Subject: [PATCH 1/2] Get loader-classname lock for calls to findLoadedClass This is related to Open J9 issue https://github.com/eclipse-openj9/openj9/issues/20444 There is a concern that the initial call to findLoadedClass by Equinox is not protected by the same loader-classname lock as the later calls to findLoadedClass/defineClass are. There are concerns that the unprotected findLoadedClass call could catch the JVM in an invalid state if there is another thread in the middle of defining the class. This change replaces the clunky way of doing loader-classname locks with a more efficient strategy. This way we can use this lock around the initial call to findLoadedClass without too much concern over the performance impact. This change also removes some of the old code that still supported Java 6. It is always assumed we now can register the class loaders as parallel capable. --- .../internal/container/KeyBasedLockStore.java | 71 +++++++++++ .../internal/loader/EquinoxClassLoader.java | 15 +-- .../internal/loader/ModuleClassLoader.java | 110 ++++++------------ 3 files changed, 111 insertions(+), 85 deletions(-) create mode 100644 bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/container/KeyBasedLockStore.java diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/container/KeyBasedLockStore.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/container/KeyBasedLockStore.java new file mode 100644 index 00000000000..e4b5e9d3828 --- /dev/null +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/container/KeyBasedLockStore.java @@ -0,0 +1,71 @@ +/******************************************************************************* + * Copyright (c) 2022, 2024 IBM Corporation and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * IBM Corporation - initial API and implementation + *******************************************************************************/ +package org.eclipse.osgi.internal.container; + +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; + +public final class KeyBasedLockStore { + + final ReferenceQueue refQueue = new ReferenceQueue<>(); + private final ConcurrentHashMap lockMap = new ConcurrentHashMap<>(); + private final Function lockCreator; + + public KeyBasedLockStore(Function lockCreator) { + this.lockCreator = lockCreator; + } + + private final class LockWeakRef extends WeakReference { + final Key key; + + public LockWeakRef(Lock referent, Key keyValue) { + super(referent, refQueue); + key = keyValue; + } + } + + public final Lock getLock(Key key) { + poll(); + LockWeakRef lockRef = lockMap.get(key); + Lock lock = lockRef != null ? lockRef.get() : null; + if (lock != null) { + return lock; + } + + lock = lockCreator.apply(key); + + while (true) { + LockWeakRef retVal = lockMap.putIfAbsent(key, new LockWeakRef(lock, key)); + if (retVal == null) { + return lock; + } + + Lock retLock = retVal.get(); + if (retLock != null) { + return retLock; + } + lockMap.remove(key, retVal); + } + } + + @SuppressWarnings("unchecked") + private final void poll() { + LockWeakRef lockRef; + while ((lockRef = (LockWeakRef) refQueue.poll()) != null) { + lockMap.remove(lockRef.key, lockRef); + } + } +} diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/EquinoxClassLoader.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/EquinoxClassLoader.java index c834e453413..b7defd1509a 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/EquinoxClassLoader.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/EquinoxClassLoader.java @@ -20,15 +20,12 @@ import org.eclipse.osgi.storage.BundleInfo.Generation; public class EquinoxClassLoader extends ModuleClassLoader { - protected static final boolean EQUINOX_REGISTERED_AS_PARALLEL; static { - boolean registered; try { - registered = ClassLoader.registerAsParallelCapable(); + ClassLoader.registerAsParallelCapable(); } catch (Throwable t) { - registered = false; + // ignore all exceptions; substrate native image fails here } - EQUINOX_REGISTERED_AS_PARALLEL = registered; } private final EquinoxConfiguration configuration; private final Debug debug; @@ -37,7 +34,6 @@ public class EquinoxClassLoader extends ModuleClassLoader { // TODO Note that PDE has internal dependency on this field type/name (bug // 267238) private final ClasspathManager manager; - private final boolean isRegisteredAsParallel; /** * Constructs a new DefaultClassLoader. @@ -55,8 +51,6 @@ public EquinoxClassLoader(ClassLoader parent, EquinoxConfiguration configuration this.delegate = delegate; this.generation = generation; this.manager = new ClasspathManager(generation, this); - this.isRegisteredAsParallel = (ModuleClassLoader.REGISTERED_AS_PARALLEL && EQUINOX_REGISTERED_AS_PARALLEL) - || this.configuration.PARALLEL_CAPABLE; } @Override @@ -69,11 +63,6 @@ public final ClasspathManager getClasspathManager() { return manager; } - @Override - public final boolean isRegisteredAsParallel() { - return isRegisteredAsParallel; - } - @Override public final BundleLoader getBundleLoader() { return delegate; diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/ModuleClassLoader.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/ModuleClassLoader.java index c586190d334..0d6e7e7ba18 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/ModuleClassLoader.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/loader/ModuleClassLoader.java @@ -25,10 +25,10 @@ import java.security.cert.Certificate; import java.util.Collection; import java.util.Enumeration; -import java.util.HashMap; import java.util.List; -import java.util.Map; +import java.util.function.Function; import org.eclipse.osgi.container.ModuleRevision; +import org.eclipse.osgi.internal.container.KeyBasedLockStore; import org.eclipse.osgi.internal.debug.Debug; import org.eclipse.osgi.internal.framework.EquinoxConfiguration; import org.eclipse.osgi.internal.loader.classpath.ClasspathEntry; @@ -61,15 +61,12 @@ public Bundle getBundle() { * ProtectionDomains when security is disabled */ protected static final PermissionCollection ALLPERMISSIONS; - protected static final boolean REGISTERED_AS_PARALLEL; static { - boolean registered; try { - registered = ClassLoader.registerAsParallelCapable(); + ClassLoader.registerAsParallelCapable(); } catch (Throwable t) { - registered = false; + // ignore all exceptions; substrate native image fails here } - REGISTERED_AS_PARALLEL = registered; } static { @@ -100,7 +97,26 @@ public DefineClassResult(Class clazz, boolean defined) { } } - private final Map classNameLocks = new HashMap<>(5); + private static final class ClassNameLock { + static final Function SUPPLIER = new Function() { + public ClassNameLock apply(String className) { + return new ClassNameLock(className); + } + }; + final String name; + + ClassNameLock(String name) { + this.name = name; + } + + @Override + public String toString() { + return "ClassNameLock: " + name; //$NON-NLS-1$ + } + } + + private final KeyBasedLockStore classNameLocks = new KeyBasedLockStore<>( + ClassNameLock.SUPPLIER); private final Object pkgLock = new Object(); /** @@ -149,12 +165,15 @@ public ModuleClassLoader(ClassLoader parent) { /** * Returns true if this class loader implementation has been registered with the - * JVM as a parallel class loader. This requires Java 7 or later. + * JVM as a parallel class loader. This requires Java 7 or later. This always + * returns true now that Java 8 is required. * * @return true if this class loader implementation has been registered with the * JVM as a parallel class loader; otherwise false is returned. */ - public abstract boolean isRegisteredAsParallel(); + public boolean isRegisteredAsParallel() { + return true; + } /** * Loads a class for the bundle. First delegate.findClass(name) is called. The @@ -284,38 +303,18 @@ public DefineClassResult defineClass(String name, byte[] classbytes, ClasspathEn // See ClasspathManager.findLocalClass(String) boolean defined = false; Class result = null; - if (isRegisteredAsParallel()) { - // lock by class name in this case - boolean initialLock = lockClassName(name); - try { - result = findLoadedClass(name); - if (result == null) { - result = defineClass(name, classbytes, 0, classbytes.length, classpathEntry.getDomain()); - defined = true; - } - } finally { - if (initialLock) { - unlockClassName(name); - } - } - } else { - // lock by class loader instance in this case - synchronized (this) { - result = findLoadedClass(name); - if (result == null) { - result = defineClass(name, classbytes, 0, classbytes.length, classpathEntry.getDomain()); - defined = true; - } + synchronized (getClassLoadingLock(name)) { + result = findLoadedClass(name); + if (result == null) { + result = defineClass(name, classbytes, 0, classbytes.length, classpathEntry.getDomain()); + defined = true; } } return new DefineClassResult(result, defined); } public Class publicFindLoaded(String classname) { - if (isRegisteredAsParallel()) { - return findLoadedClass(classname); - } - synchronized (this) { + synchronized (getClassLoadingLock(classname)) { return findLoadedClass(classname); } } @@ -427,42 +426,9 @@ public void loadFragments(Collection fragments) { getClasspathManager().loadFragments(fragments); } - private boolean lockClassName(String classname) { - synchronized (classNameLocks) { - Object lockingThread = classNameLocks.get(classname); - Thread current = Thread.currentThread(); - if (lockingThread == current) - return false; - boolean previousInterruption = Thread.interrupted(); - try { - while (true) { - if (lockingThread == null) { - classNameLocks.put(classname, current); - return true; - } - - classNameLocks.wait(); - lockingThread = classNameLocks.get(classname); - } - } catch (InterruptedException e) { - previousInterruption = true; - // must not throw LinkageError or ClassNotFoundException here because that will - // cause all threads - // to fail to load the class (see bug 490902) - throw new Error("Interrupted while waiting for classname lock: " + classname, e); //$NON-NLS-1$ - } finally { - if (previousInterruption) { - current.interrupt(); - } - } - } - } - - private void unlockClassName(String classname) { - synchronized (classNameLocks) { - classNameLocks.remove(classname); - classNameLocks.notifyAll(); - } + @Override + public Object getClassLoadingLock(String classname) { + return classNameLocks.getLock(classname); } public void close() { From 793dcc532e6b66cd8adb3fb70aa6fe5c03b774c6 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Fri, 15 Nov 2024 16:25:34 -0600 Subject: [PATCH 2/2] Stop throwing IllegalStateException The following methods can stop throwing IllegalStateExceptions Bundle.uninstall BundleContext.createFilter BundleContext.getBundle BundleContext.getProperty BundleContext.removeServiceListener BundleContext.removeBundleListener BundleContext.removeFrameworkListener BundleContext.ungetService ServiceObjects.ungetService ServiceRegistration.unregister --- .../META-INF/MANIFEST.MF | 2 +- bundles/org.eclipse.osgi.tests/pom.xml | 2 +- .../osgi/tests/bundles/BundleTests.java | 3 +- .../tests/bundles/ExceptionMessageTest.java | 24 --- .../bundles/IllegalStateExceptionTests.java | 138 ++++++++++++++++++ .../osgi/container/ModuleContainer.java | 5 +- .../internal/framework/BundleContextImpl.java | 17 +-- .../PrototypeServiceFactoryUse.java | 2 +- .../serviceregistry/ServiceObjectsImpl.java | 15 +- .../ServiceRegistrationImpl.java | 2 +- .../internal/serviceregistry/ServiceUse.java | 2 +- 11 files changed, 162 insertions(+), 50 deletions(-) create mode 100644 bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/IllegalStateExceptionTests.java diff --git a/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF b/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF index e6f242c9e86..0180cff5d86 100644 --- a/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: Core OSGi Tests Bundle-SymbolicName: org.eclipse.osgi.tests;singleton:=true -Bundle-Version: 3.20.300.qualifier +Bundle-Version: 3.22.100.qualifier Bundle-Vendor: Eclipse.org Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.29.0,4.0.0)", diff --git a/bundles/org.eclipse.osgi.tests/pom.xml b/bundles/org.eclipse.osgi.tests/pom.xml index 74175d32aee..5cd857425a7 100644 --- a/bundles/org.eclipse.osgi.tests/pom.xml +++ b/bundles/org.eclipse.osgi.tests/pom.xml @@ -19,7 +19,7 @@ org.eclipse.osgi org.eclipse.osgi.tests - 3.20.300-SNAPSHOT + 3.22.100-SNAPSHOT eclipse-test-plugin diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleTests.java index f0ab86d6a74..7f59115958d 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/BundleTests.java @@ -40,7 +40,8 @@ PlatformAdminBundleTests.class, // ListenerTests.class, // AddDynamicImportTests.class, // - BundleNativeCodeTests.class // + BundleNativeCodeTests.class, // + IllegalStateExceptionTests.class }) public class BundleTests { } diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ExceptionMessageTest.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ExceptionMessageTest.java index f00805d56e4..99d03ddcd44 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ExceptionMessageTest.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ExceptionMessageTest.java @@ -26,7 +26,6 @@ import org.eclipse.osgi.tests.OSGiTestsActivator; import org.junit.Test; import org.osgi.framework.Bundle; -import org.osgi.framework.BundleContext; import org.osgi.framework.BundleException; import org.osgi.framework.Constants; import org.osgi.framework.FrameworkUtil; @@ -52,16 +51,6 @@ public void testUninstallModuleError() throws BundleException { assertTrue("Wrong message: " + e.getMessage(), e.getMessage().endsWith(b.adapt(Module.class).toString())); } - @Test - public void testUninstallContextError() throws BundleException { - Bundle b = installer.installBundle("test"); - b.start(); - BundleContext context = b.getBundleContext(); - b.uninstall(); - IllegalStateException e = assertThrows(IllegalStateException.class, () -> context.createFilter("(a=b)")); - assertTrue("Wrong message: " + e.getMessage(), e.getMessage().endsWith(b.toString())); - } - @Test public void testStartFragmentError() throws BundleException, IOException { Map headers = new HashMap<>(); @@ -101,23 +90,10 @@ public void testUnregisterSetPropsError() throws BundleException { IllegalStateException e1 = assertThrows(IllegalStateException.class, () -> reg.setProperties(props2)); assertTrue("Wrong message: " + e1.getMessage(), e1.getMessage().endsWith(reg.toString())); - IllegalStateException e2 = assertThrows(IllegalStateException.class, () -> reg.unregister()); - assertTrue("Wrong message: " + e2.getMessage(), e2.getMessage().endsWith(reg.toString())); - IllegalStateException e3 = assertThrows(IllegalStateException.class, () -> reg.getReference()); assertTrue("Wrong message: " + e3.getMessage(), e3.getMessage().endsWith(reg.toString())); } - @Test - public void testUnregisterTwiceError() throws BundleException { - Bundle b = installer.installBundle("test"); - b.start(); - BundleContext context = b.getBundleContext(); - ServiceRegistration reg = context.registerService(Object.class, new Object(), - getDicinotary("k1", "v1")); - reg.unregister(); - } - private Dictionary getDicinotary(String key, Object value) { return FrameworkUtil.asDictionary(Collections.singletonMap(key, value)); } diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/IllegalStateExceptionTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/IllegalStateExceptionTests.java new file mode 100644 index 00000000000..b390408d069 --- /dev/null +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/IllegalStateExceptionTests.java @@ -0,0 +1,138 @@ +/******************************************************************************* + * Copyright (c) 2024 IBM Corporation and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * IBM Corporation - initial API and implementation + *******************************************************************************/ +package org.eclipse.osgi.tests.bundles; + +import static org.junit.Assert.assertNotNull; + +import org.junit.Test; +import org.osgi.framework.Bundle; +import org.osgi.framework.BundleContext; +import org.osgi.framework.BundleEvent; +import org.osgi.framework.BundleListener; +import org.osgi.framework.Constants; +import org.osgi.framework.Filter; +import org.osgi.framework.FrameworkEvent; +import org.osgi.framework.FrameworkListener; +import org.osgi.framework.ServiceEvent; +import org.osgi.framework.ServiceListener; +import org.osgi.framework.ServiceObjects; +import org.osgi.framework.ServiceReference; +import org.osgi.framework.ServiceRegistration; +import org.osgi.service.condition.Condition; + +public class IllegalStateExceptionTests extends AbstractBundleTests { + + @Test + public void testUninstall() throws Exception { + Bundle bundle = installer.installBundle("test"); + bundle.uninstall(); + // uninstall again should not throw an exception + bundle.uninstall(); + } + + @Test + public void testCreateFilter() throws Exception { + Filter testFilter = getInvalidContext().createFilter("(test=illegalstateexception)"); + assertNotNull("Null filter returned", testFilter); + } + + @Test + public void testGetBundle() throws Exception { + BundleContext invalidContext = getInvalidContext(); + + assertNotNull("Null bundle returned", invalidContext.getBundle()); + assertNotNull("Null system bundle returned", invalidContext.getBundle(Constants.SYSTEM_BUNDLE_LOCATION)); + assertNotNull("Null system bundle returned", invalidContext.getBundle(Constants.SYSTEM_BUNDLE_ID)); + } + + @Test + public void testGetProperty() throws Exception { + BundleContext invalidContext = getInvalidContext(); + + assertNotNull("Null UUID Property", invalidContext.getProperty(Constants.FRAMEWORK_UUID)); + } + + @Test + public void testRemoveServiceListener() throws Exception { + BundleContext invalidContext = getInvalidContext(); + + invalidContext.removeServiceListener(new ServiceListener() { + @Override + public void serviceChanged(ServiceEvent event) { + // nothing + } + }); + } + + @Test + public void testRemoveBundleListener() throws Exception { + BundleContext invalidContext = getInvalidContext(); + + invalidContext.removeBundleListener(new BundleListener() { + @Override + public void bundleChanged(BundleEvent event) { + // nothing + } + }); + } + + @Test + public void testFrameworkListener() throws Exception { + BundleContext invalidContext = getInvalidContext(); + + invalidContext.removeFrameworkListener(new FrameworkListener() { + @Override + public void frameworkEvent(FrameworkEvent event) { + // nothing + } + }); + } + + @Test + public void testContextUngetService() throws Exception { + BundleContext invalidContext = getInvalidContext(); + + invalidContext.ungetService(getContext().getServiceReference("org.osgi.service.condition.Condition")); + } + + @Test + public void testServiceObjectsUngetService() throws Exception { + Bundle bundle = installer.installBundle("test"); + bundle.start(); + BundleContext context = bundle.getBundleContext(); + ServiceReference ref = context.getServiceReference(Condition.class); + ServiceObjects serviceObjects = context.getServiceObjects(ref); + Condition c = serviceObjects.getService(); + bundle.stop(); + serviceObjects.ungetService(c); + } + + @Test + public void testUnregister() throws Exception { + Bundle bundle = installer.installBundle("test"); + bundle.start(); + BundleContext context = bundle.getBundleContext(); + ServiceRegistration reg = context.registerService(Condition.class, Condition.INSTANCE, null); + reg.unregister(); + reg.unregister(); + } + + public BundleContext getInvalidContext() throws Exception { + Bundle bundle = installer.installBundle("test"); + bundle.start(); + BundleContext context = bundle.getBundleContext(); + bundle.stop(); + return context; + } +} diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java index 79a038afdcd..c25faf7fc3a 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java @@ -616,7 +616,10 @@ public void uninstall(Module module) throws BundleException { module.lockStateChange(ModuleEvent.UNINSTALLED); State previousState; try { - module.checkValid(); + if (module.getState().equals(State.UNINSTALLED)) { + // no longer throwing an IllegalStateException here. + return; + } previousState = module.getState(); if (Module.ACTIVE_SET.contains(module.getState())) { try { diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/BundleContextImpl.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/BundleContextImpl.java index 9bb0028cdd9..0f39a5503c2 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/BundleContextImpl.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/BundleContextImpl.java @@ -160,8 +160,6 @@ public String getProperty(String key) { */ @Override public Bundle getBundle() { - checkValid(); - return getBundleImpl(); } @@ -305,12 +303,9 @@ public void addServiceListener(ServiceListener listener) { * method does nothing. * * @param listener The service listener to remove. - * @exception java.lang.IllegalStateException If the bundle context has stopped. */ @Override public void removeServiceListener(ServiceListener listener) { - checkValid(); - if (listener == null) { throw new IllegalArgumentException(); } @@ -354,11 +349,9 @@ public void addBundleListener(BundleListener listener) { * method does nothing. * * @param listener The bundle listener to remove. - * @exception java.lang.IllegalStateException If the bundle context has stopped. */ @Override public void removeBundleListener(BundleListener listener) { - checkValid(); if (listener == null) { throw new IllegalArgumentException(); } @@ -409,11 +402,9 @@ public void addFrameworkListener(FrameworkListener listener) { * method does nothing. * * @param listener The framework listener to remove. - * @exception java.lang.IllegalStateException If the bundle context has stopped. */ @Override public void removeFrameworkListener(FrameworkListener listener) { - checkValid(); if (listener == null) { throw new IllegalArgumentException(); } @@ -708,8 +699,6 @@ public S getService(ServiceReference reference) { */ @Override public boolean ungetService(ServiceReference reference) { - checkValid(); - return container.getServiceRegistry().ungetService(this, (ServiceReferenceImpl) reference); } @@ -1043,13 +1032,9 @@ public void dispatchEvent(Object originalListener, Object l, int action, Object * * @param filter The filter string. * @return A Filter object encapsulating the filter string. - * @exception InvalidSyntaxException If the filter parameter contains an invalid - * filter string which cannot be parsed. */ @Override public Filter createFilter(String filter) throws InvalidSyntaxException { - checkValid(); - return FilterImpl.newInstance(filter, container.getConfiguration().getDebug().DEBUG_FILTER); } @@ -1070,7 +1055,7 @@ public void checkValid() { * * @return true if the context is still valid; false otherwise */ - protected boolean isValid() { + public boolean isValid() { return valid; } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/PrototypeServiceFactoryUse.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/PrototypeServiceFactoryUse.java index 783534658d4..43e8e794120 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/PrototypeServiceFactoryUse.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/PrototypeServiceFactoryUse.java @@ -96,7 +96,7 @@ S newServiceObject() { @Override boolean releaseServiceObject(final S service) { assert isLocked(); - if ((service == null) || !serviceObjects.containsKey(service)) { + if (((service == null) || !serviceObjects.containsKey(service)) && context.isValid()) { throw new IllegalArgumentException(Msg.SERVICE_OBJECTS_UNGET_ARGUMENT_EXCEPTION); } if (debug.DEBUG_SERVICES) { diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceObjectsImpl.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceObjectsImpl.java index f71a9aab389..9efd7fec826 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceObjectsImpl.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceObjectsImpl.java @@ -16,7 +16,15 @@ import org.eclipse.osgi.internal.framework.BundleContextImpl; import org.eclipse.osgi.internal.messages.Msg; -import org.osgi.framework.*; +import org.osgi.framework.Bundle; +import org.osgi.framework.BundleContext; +import org.osgi.framework.Constants; +import org.osgi.framework.FrameworkEvent; +import org.osgi.framework.PrototypeServiceFactory; +import org.osgi.framework.ServiceException; +import org.osgi.framework.ServiceObjects; +import org.osgi.framework.ServiceReference; +import org.osgi.framework.ServiceRegistration; /** * ServiceObjects implementation. @@ -132,13 +140,14 @@ public S getService() { */ @Override public void ungetService(S service) { - user.checkValid(); boolean removed = registration.ungetService(user, ServiceConsumer.prototypeConsumer, service); if (!removed) { if (registration.isUnregistered()) { return; } - throw new IllegalArgumentException(Msg.SERVICE_OBJECTS_UNGET_ARGUMENT_EXCEPTION); + if (user.isValid()) { + throw new IllegalArgumentException(Msg.SERVICE_OBJECTS_UNGET_ARGUMENT_EXCEPTION); + } } } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java index f4e6b339a11..252f362c610 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java @@ -225,7 +225,7 @@ public void unregister() { synchronized (registry) { synchronized (registrationLock) { if (state != REGISTERED) { /* in the process of unregisterING */ - throw new IllegalStateException(Msg.SERVICE_ALREADY_UNREGISTERED_EXCEPTION + ' ' + this); + return; } /* remove this object from the service registry */ diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceUse.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceUse.java index 0a729e19fa4..e27442f925f 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceUse.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/serviceregistry/ServiceUse.java @@ -149,7 +149,7 @@ S newServiceObject() { */ /* @GuardedBy("getLock()") */ boolean releaseServiceObject(final S service) { - if ((service == null) || (service != getCachedService())) { + if ((service == null) || (service != getCachedService()) && context.isValid()) { throw new IllegalArgumentException(Msg.SERVICE_OBJECTS_UNGET_ARGUMENT_EXCEPTION); } if (debug.DEBUG_SERVICES) {