tjwatson commented on a change in pull request #81:
URL: https://github.com/apache/felix-dev/pull/81#discussion_r673979834
##########
File path:
scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
##########
@@ -328,7 +328,7 @@ public void dispose()
private Map<RefPair<?, ?>, Object> createNewFieldHandlerMap()
{
- return new TreeMap<>(
+ return new ConcurrentSkipListMap<>(
Review comment:
While I think this will avoid the exception in TreeMap, I do not think
it is a complete solution. In
`org.apache.felix.scr.impl.inject.field.FieldHandler.updateField(METHOD_TYPE,
Object, BindParameters)` we have the following code:
```java
if ( mType == METHOD_TYPE.UNBIND )
{
if ( this.metadata.isOptional() && !this.metadata.isStatic()
)
{
// we only reset if it was previously set with this value
if (
bp.getComponentContext().getBoundValues(metadata.getName()).size() == 1 )
{
this.setFieldValue(componentInstance, null);
}
}
bp.getComponentContext().getBoundValues(metadata.getName()).remove(refPair);
}
```
`getBoundValues` call here will return this `ConcurrentSkipListMap`. If
multiple threads are entering this block then I think we have an issue with
never calling `this.setFieldValue(componentInstance, null);` when two threads
enter the `size() ==1` check and find the size to be 2. Both threads will
remove their own refPair from the Map but none will null out the field value
when the last is removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
