Skip to content

Commit

Permalink
Stop throwing IllegalStateException
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tjwatson committed Dec 2, 2024
1 parent a74bbe6 commit 793dcc5
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 50 deletions.
2 changes: 1 addition & 1 deletion bundles/org.eclipse.osgi.tests/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.osgi.tests/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
</parent>
<groupId>org.eclipse.osgi</groupId>
<artifactId>org.eclipse.osgi.tests</artifactId>
<version>3.20.300-SNAPSHOT</version>
<version>3.22.100-SNAPSHOT</version>
<packaging>eclipse-test-plugin</packaging>

<properties>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
PlatformAdminBundleTests.class, //
ListenerTests.class, //
AddDynamicImportTests.class, //
BundleNativeCodeTests.class //
BundleNativeCodeTests.class, //
IllegalStateExceptionTests.class
})
public class BundleTests {
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<>();
Expand Down Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,6 @@ public String getProperty(String key) {
*/
@Override
public Bundle getBundle() {
checkValid();

return getBundleImpl();
}

Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 793dcc5

Please sign in to comment.