Stop throwing IllegalStateException

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 {
 }

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<String, String> 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<Object> reg = context.registerService(Object.class, new Object(),
-				getDicinotary("k1", "v1"));
-		reg.unregister();
-	}
-
 	private Dictionary<String, Object> getDicinotary(String key, Object value) {
 		return FrameworkUtil.asDictionary(Collections.singletonMap(key, value));
 	}

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<Condition> ref = context.getServiceReference(Condition.class);
+		ServiceObjects<Condition> 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<Condition> 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;
+	}
+}

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 {

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> S getService(ServiceReference<S> 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;
 	}
 

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) {

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);
+			}
 		}
 	}
 

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 */

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) {