From a623362045459e3aef6b166fdb06bb8cacf3c4d0 Mon Sep 17 00:00:00 2001 From: Juergen Albert Date: Wed, 11 Sep 2024 16:38:40 +0200 Subject: [PATCH] avoids IllegalArgumentException on unget fixes https://issues.apache.org/jira/browse/FELIX-6726 ungetServiceObject may be called twice in some cases, which causes an IllegalArgumentException to be thrown and logged. Signed-off-by: Juergen Albert --- .../helper/ComponentServiceObjectsHelper.java | 3 +- .../scr/impl/manager/DependencyManager.java | 5 +- .../scr/integration/ComponentTestBase.java | 17 ++-- .../felix/scr/integration/Felix6726Test.java | 81 +++++++++++++++++++ .../integration/components/felix6726/A.java | 23 ++++++ .../integration/components/felix6726/B.java | 23 ++++++ .../components/felix6726/BImpl.java | 24 ++++++ .../components/felix6726/Consumer.java | 29 +++++++ .../resources/integration_test_FELIX_6726.xml | 44 ++++++++++ 9 files changed, 236 insertions(+), 13 deletions(-) create mode 100644 scr/src/test/java/org/apache/felix/scr/integration/Felix6726Test.java create mode 100644 scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/A.java create mode 100644 scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/B.java create mode 100644 scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/BImpl.java create mode 100644 scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/Consumer.java create mode 100644 scr/src/test/resources/integration_test_FELIX_6726.xml diff --git a/scr/src/main/java/org/apache/felix/scr/impl/helper/ComponentServiceObjectsHelper.java b/scr/src/main/java/org/apache/felix/scr/impl/helper/ComponentServiceObjectsHelper.java index 7f32e6c946..f8524b1fe5 100644 --- a/scr/src/main/java/org/apache/felix/scr/impl/helper/ComponentServiceObjectsHelper.java +++ b/scr/src/main/java/org/apache/felix/scr/impl/helper/ComponentServiceObjectsHelper.java @@ -206,10 +206,9 @@ public void ungetService(final T service) final ServiceObjects so = this.serviceObjects; if ( so != null ) { - boolean remove; synchronized ( instances ) { - remove = instances.remove(service); + instances.remove(service); } so.ungetService(service); } diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java index 031b2c1c01..ebe59739bb 100644 --- a/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java +++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java @@ -1959,10 +1959,9 @@ void close(ComponentContextImpl componentContext, EdgeInfo edgeInfo) if (doUnbind && !boundRef.isFailed()) { invokeUnbindMethod(componentContext, boundRef, trackingCount.get(), edgeInfo); + } else { + boundRef.ungetServiceObject(componentContext); } - - boundRef.ungetServiceObject(componentContext); - } latch.countDown(); } diff --git a/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java b/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java index 39724c1b0f..7bf2b23f28 100644 --- a/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java +++ b/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java @@ -164,6 +164,7 @@ public TestProbeBuilder extendProbe(TestProbeBuilder builder) + "org.apache.felix.scr.integration.components.felix4984," + "org.apache.felix.scr.integration.components.felix5248," + "org.apache.felix.scr.integration.components.felix5276," + + "org.apache.felix.scr.integration.components.felix6726," + "org.apache.felix.scr.integration.components.metadata.cache" ); builder.setHeader( "Import-Package", "org.apache.felix.scr.component" ); builder.setHeader( "Bundle-ManifestVersion", "2" ); @@ -750,12 +751,12 @@ protected InputStream createBundleInputStream(final String descriptorFile, final InputStream bundleStream = bundle().add("OSGI-INF/components.xml", getClass().getResource( descriptorFile ) ) - .set( Constants.BUNDLE_SYMBOLICNAME, symbolicName ).set( Constants.BUNDLE_VERSION, version ).set( - Constants.IMPORT_PACKAGE, componentPackage ).set( "Service-Component", "OSGI-INF/components.xml" ).set( - Constants.REQUIRE_CAPABILITY, - ExtenderNamespace.EXTENDER_NAMESPACE - + ";filter:=\"(&(osgi.extender=osgi.component)(version>=1.3)(!(version>=2.0)))\"" ).build( - withBnd() ); + .set( Constants.BUNDLE_SYMBOLICNAME, symbolicName ) + .set( Constants.BUNDLE_VERSION, version ) + .set( Constants.IMPORT_PACKAGE, componentPackage ) + .set( "Service-Component", "OSGI-INF/components.xml" ) + .set( Constants.REQUIRE_CAPABILITY, ExtenderNamespace.EXTENDER_NAMESPACE + ";filter:=\"(&(osgi.extender=osgi.component)(version>=1.3)(!(version>=2.0)))\"" ) + .build( withBnd() ); return bundleStream; } @@ -898,8 +899,8 @@ public void start() { m_realOut = System.out; m_realErr = System.err; - System.setOut( new NullStdout() ); - System.setErr( new NullStdout() ); +// System.setOut( new NullStdout() ); +// System.setErr( new NullStdout() ); m_logThread = new Thread( this ); m_logThread.start(); } diff --git a/scr/src/test/java/org/apache/felix/scr/integration/Felix6726Test.java b/scr/src/test/java/org/apache/felix/scr/integration/Felix6726Test.java new file mode 100644 index 0000000000..87801a2752 --- /dev/null +++ b/scr/src/test/java/org/apache/felix/scr/integration/Felix6726Test.java @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.felix.scr.integration; + +import static org.junit.Assert.fail; + +import javax.inject.Inject; + +import org.apache.felix.scr.integration.components.felix6726.Consumer; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.ops4j.pax.exam.junit.PaxExam; +import org.osgi.framework.BundleContext; +import org.osgi.service.component.runtime.dto.ComponentConfigurationDTO; + +/** + * This test validates the FELIX-6726 issue. + */ +@RunWith(PaxExam.class) +public class Felix6726Test extends ComponentTestBase +{ + static + { + // uncomment to enable debugging of this test class + // paxRunnerVmOption = DEBUG_VM_OPTION; + descriptorFile = "/integration_test_FELIX_6726.xml"; + COMPONENT_PACKAGE = COMPONENT_PACKAGE + ".felix6726"; + restrictedLogging = false; + //comment to get debug logging if the test fails. + DS_LOGLEVEL = "debug"; + //paxRunnerVmOption = DEBUG_VM_OPTION; + } + + @Inject + protected BundleContext bundleContext; + + protected static void delay(int secs) + { + try + { + Thread.sleep(secs * 1000); + } + catch (InterruptedException ie) + { + } + } + + /** + * This Test actually never fails, but it will provoke the {@link IllegalStateException} to be logged by SCR + */ + @Test + public void test_IllegalStateExceptionLogging() + { + final ComponentConfigurationDTO bImplDTO = findComponentConfigurationByName("felix.6726.B", 4); + final ComponentConfigurationDTO consumerDTO = findComponentConfigurationByName( "felix.6726.Consumer", 4); + + Consumer consumer = getServiceFromConfiguration(consumerDTO, Consumer.class); + try { + ungetServiceFromConfiguration(consumerDTO, Consumer.class); + } catch (IllegalStateException e) { + fail(); + } + + } +} diff --git a/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/A.java b/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/A.java new file mode 100644 index 0000000000..43cba258fa --- /dev/null +++ b/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/A.java @@ -0,0 +1,23 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.felix.scr.integration.components.felix6726; + +public interface A { + +} diff --git a/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/B.java b/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/B.java new file mode 100644 index 0000000000..40ccca237f --- /dev/null +++ b/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/B.java @@ -0,0 +1,23 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.felix.scr.integration.components.felix6726; + +public interface B extends A { + +} diff --git a/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/BImpl.java b/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/BImpl.java new file mode 100644 index 0000000000..17ce7e9d0b --- /dev/null +++ b/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/BImpl.java @@ -0,0 +1,24 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.felix.scr.integration.components.felix6726; + +public class BImpl implements B +{ + +} diff --git a/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/Consumer.java b/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/Consumer.java new file mode 100644 index 0000000000..3f30ca3111 --- /dev/null +++ b/scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/Consumer.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.felix.scr.integration.components.felix6726; + +public class Consumer { + + A reference; + + public void activate() { + System.out.println("Hallo World"); + } + +} diff --git a/scr/src/test/resources/integration_test_FELIX_6726.xml b/scr/src/test/resources/integration_test_FELIX_6726.xml new file mode 100644 index 0000000000..2fb4f7e700 --- /dev/null +++ b/scr/src/test/resources/integration_test_FELIX_6726.xml @@ -0,0 +1,44 @@ + + + + + + + + + + + + + + + + + + + \ No newline at end of file