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 <[email protected]>
apache · Sep 11, 2024 · a623362 · a623362
1 parent 396de83
commit a623362
Show file tree

Hide file tree

Showing 9 changed files with 236 additions and 13 deletions.
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
@@ -206,10 +206,9 @@ public void ungetService(final T service)
             final ServiceObjects<T> 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
@@ -1959,10 +1959,9 @@ void close(ComponentContextImpl<S> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,44 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+    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.
+-->
+<components>
+    <scr:component name="felix.6726.B"
+                   xmlns:scr="http://www.osgi.org/xmlns/scr/v1.3.0"
+                   enabled="true">
+        <implementation class="org.apache.felix.scr.integration.components.felix6726.BImpl" />
+        <service scope="prototype">
+            <provide interface="org.apache.felix.scr.integration.components.felix6726.B" />
+            <provide interface="org.apache.felix.scr.integration.components.felix6726.A" />
+        </service>
+    </scr:component>
+
+    <scr:component name="felix.6726.Consumer"
+                   xmlns:scr="http://www.osgi.org/xmlns/scr/v1.3.0"
+                   activate="activate"
+                   enabled="true">
+        <implementation class="org.apache.felix.scr.integration.components.felix6726.Consumer" />
+        <service scope="prototype">
+            <provide interface="org.apache.felix.scr.integration.components.felix6726.Consumer" />
+        </service>
+        <reference name="reference" 
+                   interface="org.apache.felix.scr.integration.components.felix6726.A"
+                   scope="prototype_required" 
+                   field="reference"/>
+    </scr:component>
+</components>