Hi Oli,
I trust you with this and definitely I checked out that the order in which the 
methods are called differ here with a modified method. I haven’t checked 
whether the same object instance is reused or not.

You just need be aware of potential multithreading issues as the component is 
not stopped while being reconfigured.
I think both variables “configuration” and “skipEntityIdsWithVersions” need to 
be made volatile which might neglect the performance advantage, as otherwise 
other threads might not see the updates (similar to what bnd actually checks 
for dynamic field references).
Compare also with 
http://docs.osgi.org/specification/osgi.cmpn/8.0.0/service.component.html#service.component-reference.field.option

"For a dynamic reference, the field must be declared with the volatile modifier 
so that field value changes made by SCR are visible to other threads. If the 
field is not declared with the volatile modifier, SCR must log an error message 
with the Log Service, if present, and the field must not be modified.”

Although this is not stated for a modified method I think the same 
circumstances apply here!

Konrad


> On 28. Jul 2022, at 15:04, Oliver Lietz <[email protected]> wrote:
> 
> On Wednesday, 20 July 2022 10:41:04 CEST Oliver Lietz wrote:
>> On Tuesday, 19 July 2022 08:27:23 CEST Konrad Windszus wrote:
>>> +1,
>>> 
>>> I don’t think that the changes for
>>> https://issues.apache.org/jira/browse/SLING-11463
>>> <https://issues.apache.org/jira/browse/SLING-11463> were actually
>>> necessary
>>> as a DS component is reactivated if a configuration is changed and doesn’t
>>> have a modified method. Compare with
>>> https://github.com/apache/sling-org-apache-sling-installer-hc/commit/82096
>>> 7
>>> 59de3e5d0b791a1f45c8fe2e759b5ae391#r78840724
>>> <https://github.com/apache/sling-org-apache-sling-installer-hc/commit/8209
>>> 6
>>> 759de3e5d0b791a1f45c8fe2e759b5ae391#r78840724>.
>>> 
>>> IMHO adding a modified just allows to reconfigure while the component is
>>> running which is not necessary here. Konrad
>> 
>> That is not true for Felix at least. The component will not be reactivated
>> but recreated. Felix will create a complete new instance if modified is
>> missing which causes unnecessary fluctuation.
> 
> I've added some more unit tests and an integration test (which increased 
> coverage from ~20% to ~90%). If you have doubt I can quickly add an IT to 
> proof my statement from above.
> 
> O.
> 
> [...]

Reply via email to