This is an automated email from the ASF dual-hosted git repository.
tjwatson pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/felix-dev.git
The following commit(s) were added to refs/heads/master by this push:
new cf2cf3c Introduced thread-safety while updating field during
deactivation phase
new 6695a8a Merge pull request #81 from amitjoy/bug/FELIX-6439/npe-treemap
cf2cf3c is described below
commit cf2cf3c57801944650ca846f452adf0b0d3755da
Author: Amit Kumar Mondal <[email protected]>
AuthorDate: Thu Jul 22 14:46:07 2021 +0200
Introduced thread-safety while updating field during deactivation phase
The problem occurred when multiple injected services tried to be removed
from the component's injection. It is highly likely because of the usage of
TreeMap in such concurrent environment. Since TreeMap has never been written to
be used in a concurrent environment, we should use ConcurrentSkipListMap
instead as all the features of TreeMap have been introduced in
ConcurrentSkipListMap and it is very much suitable for concurrent environment.
Introduced thread-safety while updating field during deactivation phase
---
.../apache/felix/scr/impl/inject/field/FieldHandler.java | 16 ++++++++++------
.../felix/scr/impl/manager/ComponentContextImpl.java | 13 ++-----------
2 files changed, 12 insertions(+), 17 deletions(-)
diff --git
a/scr/src/main/java/org/apache/felix/scr/impl/inject/field/FieldHandler.java
b/scr/src/main/java/org/apache/felix/scr/impl/inject/field/FieldHandler.java
index 49f4964..541e88b 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/inject/field/FieldHandler.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/inject/field/FieldHandler.java
@@ -25,6 +25,7 @@ import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CopyOnWriteArraySet;
@@ -42,7 +43,6 @@ import org.apache.felix.scr.impl.logger.ComponentLogger;
import org.apache.felix.scr.impl.logger.InternalLogger.Level;
import org.apache.felix.scr.impl.metadata.ReferenceMetadata;
import org.osgi.framework.BundleContext;
-import org.osgi.service.log.LogLevel;
/**
* Handler for field references
@@ -186,15 +186,19 @@ public class FieldHandler
// unbind needs only be done, if reference is dynamic and
optional
if ( mType == METHOD_TYPE.UNBIND )
{
- if ( this.metadata.isOptional() && !this.metadata.isStatic() )
+ Map<RefPair<?, ?>, Object> boundValues =
bp.getComponentContext().getBoundValues(metadata.getName());
+ synchronized (boundValues)
{
- // we only reset if it was previously set with this value
- if (
bp.getComponentContext().getBoundValues(metadata.getName()).size() == 1 )
+ if ( this.metadata.isOptional() &&
!this.metadata.isStatic() )
{
- this.setFieldValue(componentInstance, null);
+ // we only reset if it was previously set with this
value
+ if ( boundValues.size() == 1 )
+ {
+ this.setFieldValue(componentInstance, null);
+ }
}
+ boundValues.remove(refPair);
}
-
bp.getComponentContext().getBoundValues(metadata.getName()).remove(refPair);
}
// updated needs only be done, if the value type is map or tuple
// If it's a dynamic reference, the value can be updated
diff --git
a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
index 3204e18..faaa498 100644
---
a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
+++
b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
@@ -19,7 +19,7 @@
package org.apache.felix.scr.impl.manager;
-import java.util.Comparator;
+import java.util.Collections;
import java.util.Dictionary;
import java.util.HashMap;
import java.util.Map;
@@ -328,15 +328,6 @@ public class ComponentContextImpl<S> implements
ScrComponentContext {
private Map<RefPair<?, ?>, Object> createNewFieldHandlerMap()
{
- return new TreeMap<>(
- new Comparator<RefPair<?, ?>>()
- {
-
- @Override
- public int compare(final RefPair<?, ?> o1, final RefPair<?, ?>
o2)
- {
- return o1.getRef().compareTo(o2.getRef());
- }
- });
+ return Collections.synchronizedMap(new TreeMap<>((o1, o2) ->
o1.getRef().compareTo(o2.getRef())));
}
}
