[jira] [Commented] (FELIX-3864) Synchronized injection

["Marcel Offermans (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FELIX-3864?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13564117#comment-13564117 ] 

Marcel Offermans commented on FELIX-3864:
-----------------------------------------

Pierre De Rop and I discussed this issue over the last couple of days, and I would like to summarise our conclusion:

In short, we do not want to add such a feature, because we feel strongly about not holding locks while calling code we don't know (in components, other services or the OSGi framework itself). These lead to deadlocks that are very difficult to find and fix.

The synchronization you mention in ComponentImpl uses a private instance (SYNC) for that exact reason. We can be sure nobody else holds that lock. The reason we set the field inside a synchronized block is to make it visible immediately. This came from a discussion we had with BJ Hargrave years ago on this topic, and the final conclusion was that setting fields in instances works best if you a) make the field volatile (which is what we therefore recommend for all injected fields) and b) use the construction mentioned above.

In general, if you have a component that uses a service and relies on some form of state between multiple calls to that service, you have to properly synchronize that yourself because I cannot see how we can make assumptions about them that are always valid.
                
> Synchronized injection
> ----------------------
>
>                 Key: FELIX-3864
>                 URL: https://issues.apache.org/jira/browse/FELIX-3864
>             Project: Felix
>          Issue Type: Improvement
>          Components: Dependency Manager
>    Affects Versions: dependencymanager-3.0.0
>            Reporter: Tuomas Kiviaho
>
> I see myself repeating the following pattern quite often
> {code}
>    @ServiceDependency( removed = "removed", required=true  )
>     synchronized void added(LogService logService )
>     {
>         this.logService = logService;
>     }
>     synchronized void removed(LogService logService )
>     {
>         this.logService = null;
>     }
> {code}
> Would it be possible to get optional synchronization support directly from dependency manager. I noticed that there's a sort of synchronization already taking place at ComponentImpl
> {code}
>   private void configureImplementation(Class clazz, Object instance, String instanceName) {
>   ...
> 		                        synchronized (SYNC) {
> 		                            field.set(serviceInstance, instance);
> 		                        }
> {code}
> I propose adding of 'boolean synchronization default false' attribute to @*Dependency which might contain a notion that this applies only to field injection. If syncronization between component's field injections is not needed then SYNC could be replaced with serviceInstance whenever the attribute is set.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira