Skip to content

Commit 6dca4b8

Browse files
juergen-albertjuergen-albert
committed
avoids IllegalArgumentException and memory hoarding
ungetServiceObject may be called twice in some cases, which causes an IllegalArgumentException to be thrown and logged. It furthermore causes to recreated ComponentServiceObjects to be created and never cleaned up afterwards. Signed-off-by: Juergen Albert <j.albert@data-in-motion.biz>
1 parent 99f28ae commit 6dca4b8

File tree

9 files changed

+236
-13
lines changed

9 files changed

+236
-13
lines changed

‎scr/src/main/java/org/apache/felix/scr/impl/helper/ComponentServiceObjectsHelper.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,9 @@ public void ungetService(final T service)
206206
final ServiceObjects<T> so = this.serviceObjects;
207207
if ( so != null )
208208
{
209-
boolean remove;
210209
synchronized ( instances )
211210
{
212-
remove = instances.remove(service);
211+
instances.remove(service);
213212
}
214213
so.ungetService(service);
215214
}

‎scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1959,10 +1959,9 @@ void close(ComponentContextImpl<S> componentContext, EdgeInfo edgeInfo)
19591959
if (doUnbind && !boundRef.isFailed())
19601960
{
19611961
invokeUnbindMethod(componentContext, boundRef, trackingCount.get(), edgeInfo);
1962+
} else {
1963+
boundRef.ungetServiceObject(componentContext);
19621964
}
1963-
1964-
boundRef.ungetServiceObject(componentContext);
1965-
19661965
}
19671966
latch.countDown();
19681967
}

‎scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ public TestProbeBuilder extendProbe(TestProbeBuilder builder)
164164
+ "org.apache.felix.scr.integration.components.felix4984,"
165165
+ "org.apache.felix.scr.integration.components.felix5248,"
166166
+ "org.apache.felix.scr.integration.components.felix5276,"
167+
+ "org.apache.felix.scr.integration.components.felix6726,"
167168
+ "org.apache.felix.scr.integration.components.metadata.cache" );
168169
builder.setHeader( "Import-Package", "org.apache.felix.scr.component" );
169170
builder.setHeader( "Bundle-ManifestVersion", "2" );
@@ -750,12 +751,12 @@ protected InputStream createBundleInputStream(final String descriptorFile,
750751
final InputStream bundleStream = bundle().add("OSGI-INF/components.xml",
751752
getClass().getResource( descriptorFile ) )
752753

753-
.set( Constants.BUNDLE_SYMBOLICNAME, symbolicName ).set( Constants.BUNDLE_VERSION, version ).set(
754-
Constants.IMPORT_PACKAGE, componentPackage ).set( "Service-Component", "OSGI-INF/components.xml" ).set(
755-
Constants.REQUIRE_CAPABILITY,
756-
ExtenderNamespace.EXTENDER_NAMESPACE
757-
+ ";filter:=\"(&(osgi.extender=osgi.component)(version>=1.3)(!(version>=2.0)))\"" ).build(
758-
withBnd() );
754+
.set( Constants.BUNDLE_SYMBOLICNAME, symbolicName )
755+
.set( Constants.BUNDLE_VERSION, version )
756+
.set( Constants.IMPORT_PACKAGE, componentPackage )
757+
.set( "Service-Component", "OSGI-INF/components.xml" )
758+
.set( Constants.REQUIRE_CAPABILITY, ExtenderNamespace.EXTENDER_NAMESPACE + ";filter:=\"(&(osgi.extender=osgi.component)(version>=1.3)(!(version>=2.0)))\"" )
759+
.build( withBnd() );
759760
return bundleStream;
760761
}
761762

@@ -898,8 +899,8 @@ public void start()
898899
{
899900
m_realOut = System.out;
900901
m_realErr = System.err;
901-
System.setOut( new NullStdout() );
902-
System.setErr( new NullStdout() );
902+
// System.setOut( new NullStdout() );
903+
// System.setErr( new NullStdout() );
903904
m_logThread = new Thread( this );
904905
m_logThread.start();
905906
}

‎scr/src/test/java/org/apache/felix/scr/integration/Felix6726Test.java‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.felix.scr.integration;
20+
21+
import static org.junit.Assert.fail;
22+
23+
import javax.inject.Inject;
24+
25+
import org.apache.felix.scr.integration.components.felix6726.Consumer;
26+
import org.junit.Test;
27+
import org.junit.runner.RunWith;
28+
import org.ops4j.pax.exam.junit.PaxExam;
29+
import org.osgi.framework.BundleContext;
30+
import org.osgi.service.component.runtime.dto.ComponentConfigurationDTO;
31+
32+
/**
33+
* This test validates the FELIX-6726 issue.
34+
*/
35+
@RunWith(PaxExam.class)
36+
public class Felix6726Test extends ComponentTestBase
37+
{
38+
static
39+
{
40+
// uncomment to enable debugging of this test class
41+
// paxRunnerVmOption = DEBUG_VM_OPTION;
42+
descriptorFile = "/integration_test_FELIX_6726.xml";
43+
COMPONENT_PACKAGE = COMPONENT_PACKAGE + ".felix6726";
44+
restrictedLogging = false;
45+
//comment to get debug logging if the test fails.
46+
DS_LOGLEVEL = "debug";
47+
//paxRunnerVmOption = DEBUG_VM_OPTION;
48+
}
49+
50+
@Inject
51+
protected BundleContext bundleContext;
52+
53+
protected static void delay(int secs)
54+
{
55+
try
56+
{
57+
Thread.sleep(secs * 1000);
58+
}
59+
catch (InterruptedException ie)
60+
{
61+
}
62+
}
63+
64+
/**
65+
* This Test actually never fails, but it will provoke the {@link IllegalStateException} to be logged by SCR
66+
*/
67+
@Test
68+
public void test_IllegalStateExceptionLogging()
69+
{
70+
final ComponentConfigurationDTO bImplDTO = findComponentConfigurationByName("felix.6726.B", 4);
71+
final ComponentConfigurationDTO consumerDTO = findComponentConfigurationByName( "felix.6726.Consumer", 4);
72+
73+
Consumer consumer = getServiceFromConfiguration(consumerDTO, Consumer.class);
74+
try {
75+
ungetServiceFromConfiguration(consumerDTO, Consumer.class);
76+
} catch (IllegalStateException e) {
77+
fail();
78+
}
79+
80+
}
81+
}

‎scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/A.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.felix.scr.integration.components.felix6726;
20+
21+
public interface A {
22+
23+
}

‎scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/B.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.felix.scr.integration.components.felix6726;
20+
21+
public interface B extends A {
22+
23+
}

‎scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/BImpl.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.felix.scr.integration.components.felix6726;
20+
21+
public class BImpl implements B
22+
{
23+
24+
}

‎scr/src/test/java/org/apache/felix/scr/integration/components/felix6726/Consumer.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.felix.scr.integration.components.felix6726;
20+
21+
public class Consumer {
22+
23+
A reference;
24+
25+
public void activate() {
26+
System.out.println("Hallo World");
27+
}
28+
29+
}

‎scr/src/test/resources/integration_test_FELIX_6726.xml‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
Licensed to the Apache Software Foundation (ASF) under one
4+
or more contributor license agreements. See the NOTICE file
5+
distributed with this work for additional information
6+
regarding copyright ownership. The ASF licenses this file
7+
to you under the Apache License, Version 2.0 (the
8+
"License"); you may not use this file except in compliance
9+
with the License. You may obtain a copy of the License at
10+
11+
http://www.apache.org/licenses/LICENSE-2.0
12+
13+
Unless required by applicable law or agreed to in writing,
14+
software distributed under the License is distributed on an
15+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
KIND, either express or implied. See the License for the
17+
specific language governing permissions and limitations
18+
under the License.
19+
-->
20+
<components>
21+
<scr:component name="felix.6726.B"
22+
xmlns:scr="http://www.osgi.org/xmlns/scr/v1.3.0"
23+
enabled="true">
24+
<implementation class="org.apache.felix.scr.integration.components.felix6726.BImpl" />
25+
<service scope="prototype">
26+
<provide interface="org.apache.felix.scr.integration.components.felix6726.B" />
27+
<provide interface="org.apache.felix.scr.integration.components.felix6726.A" />
28+
</service>
29+
</scr:component>
30+
31+
<scr:component name="felix.6726.Consumer"
32+
xmlns:scr="http://www.osgi.org/xmlns/scr/v1.3.0"
33+
activate="activate"
34+
enabled="true">
35+
<implementation class="org.apache.felix.scr.integration.components.felix6726.Consumer" />
36+
<service scope="prototype">
37+
<provide interface="org.apache.felix.scr.integration.components.felix6726.Consumer" />
38+
</service>
39+
<reference name="reference"
40+
interface="org.apache.felix.scr.integration.components.felix6726.A"
41+
scope="prototype_required"
42+
field="reference"/>
43+
</scr:component>
44+
</components>

0 commit comments

Comments
 (0)