You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Felix Meschberger (JIRA)" <ji...@apache.org> on 2013/02/25 09:54:14 UTC
[jira] [Commented] (SLING-2743) Activator.getLoginModules in
Jackrabbit server bundle should be synchronized
[ https://issues.apache.org/jira/browse/SLING-2743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13585731#comment-13585731 ]
Felix Meschberger commented on SLING-2743:
------------------------------------------
This is IMHO not required because the bundle activator is never concurrently called by more than one thread.
Also the lock is dangerous because the start method is called by the framework which itself holds a lock on the bundle while starting it and the start method calls back into the framework.
> Activator.getLoginModules in Jackrabbit server bundle should be synchronized
> ----------------------------------------------------------------------------
>
> Key: SLING-2743
> URL: https://issues.apache.org/jira/browse/SLING-2743
> Project: Sling
> Issue Type: Bug
> Components: JCR
> Affects Versions: JCR Jackrabbit Server 2.1.0
> Reporter: Ian Boston
> Assignee: Ian Boston
> Attachments: SLING-2743.patch
>
>
> Findbugs is reporting that the initialization of the static moduleCache, loginModuleTracker is unsafe in a threaded environment. The method should probably be synchronized to ensure that only one thread accesses the method at a time.
> FindBug Report:
> Bug: Incorrect lazy initialization and update of static field org.apache.sling.jcr.jackrabbit.server.impl.Activator.moduleCache in org.apache.sling.jcr.jackrabbit.server.impl.Activator.getLoginModules()
> This method contains an unsynchronized lazy initialization of a static field. After the field is set, the object stored into that location is further updated or accessed. The setting of the field is visible to other threads as soon as it is set. If the futher accesses in the method that set the field serve to initialize the object, then you have a very serious multithreading bug, unless something else prevents any other thread from accessing the stored object until it is fully initialized.
> Even if you feel confident that the method is never called by multiple threads, it might be better to not set the static field until the value you are setting it to is fully populated/initialized.
> Confidence: High, Rank: Scary (6)
> Pattern: LI_LAZY_INIT_UPDATE_STATIC
> Type: LI, Category: MT_CORRECTNESS (Multithreaded correctness)
--
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
Re: [jira] [Created] (SLING-2743) Activator.getLoginModules in
Jackrabbit server bundle should be synchronized
Posted by Felix Meschberger <fm...@adobe.com>.
Done.
Regards
Felix
Am 25.02.2013 um 10:27 schrieb Ian Boston:
> Thanks for the feedback. Please feel free to close wont fix, or if you
> don't get round to it I'll close tomorrow. I think I need to revert part of
> another patch tomorrow if activate and deactivate are single threaded.
>
> Ian
>
> On Monday, February 25, 2013, Felix Meschberger (JIRA) wrote:
>
>>
>> [
>> https://issues.apache.org/jira/browse/SLING-2743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13585731#comment-13585731]
>>
>> Felix Meschberger commented on SLING-2743:
>> ------------------------------------------
>>
>> This is IMHO not required because the bundle activator is never
>> concurrently called by more than one thread.
>>
>> Also the lock is dangerous because the start method is called by the
>> framework which itself holds a lock on the bundle while starting it and the
>> start method calls back into the framework.
>>
>>> Activator.getLoginModules in Jackrabbit server bundle should be
>> synchronized
>>>
>> ----------------------------------------------------------------------------
>>>
>>> Key: SLING-2743
>>> URL: https://issues.apache.org/jira/browse/SLING-2743
>>> Project: Sling
>>> Issue Type: Bug
>>> Components: JCR
>>> Affects Versions: JCR Jackrabbit Server 2.1.0
>>> Reporter: Ian Boston
>>> Assignee: Ian Boston
>>> Attachments: SLING-2743.patch
>>>
>>>
>>> Findbugs is reporting that the initialization of the static moduleCache,
>> loginModuleTracker is unsafe in a threaded environment. The method should
>> probably be synchronized to ensure that only one thread accesses the method
>> at a time.
>>> FindBug Report:
>>> Bug: Incorrect lazy initialization and update of static field
>> org.apache.sling.jcr.jackrabbit.server.impl.Activator.moduleCache in
>> org.apache.sling.jcr.jackrabbit.server.impl.Activator.getLoginModules()
>>> This method contains an unsynchronized lazy initialization of a static
>> field. After the field is set, the object stored into that location is
>> further updated or accessed. The setting of the field is visible to other
>> threads as soon as it is set. If the futher accesses in the method that set
>> the field serve to initialize the object, then you have a very serious
>> multithreading bug, unless something else prevents any other thread from
>> accessing the stored object until it is fully initialized.
>>> Even if you feel confident that the method is never called by multiple
>> threads, it might be better to not set the static field until the value you
>> are setting it to is fully populated/initialized.
>>> Confidence: High, Rank: Scary (6)
>>> Pattern: LI_LAZY_INIT_UPDATE_STATIC
>>> Type: LI, Category: MT_CORRECTNESS (Multithreaded correctness)
>>
>> --
>> 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
>>
--
Felix Meschberger | Principal Scientist | Adobe
Re: [jira] [Created] (SLING-2743) Activator.getLoginModules in
Jackrabbit server bundle should be synchronized
Posted by Ian Boston <ie...@tfd.co.uk>.
Thanks for the feedback. Please feel free to close wont fix, or if you
don't get round to it I'll close tomorrow. I think I need to revert part of
another patch tomorrow if activate and deactivate are single threaded.
Ian
On Monday, February 25, 2013, Felix Meschberger (JIRA) wrote:
>
> [
> https://issues.apache.org/jira/browse/SLING-2743?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13585731#comment-13585731]
>
> Felix Meschberger commented on SLING-2743:
> ------------------------------------------
>
> This is IMHO not required because the bundle activator is never
> concurrently called by more than one thread.
>
> Also the lock is dangerous because the start method is called by the
> framework which itself holds a lock on the bundle while starting it and the
> start method calls back into the framework.
>
> > Activator.getLoginModules in Jackrabbit server bundle should be
> synchronized
> >
> ----------------------------------------------------------------------------
> >
> > Key: SLING-2743
> > URL: https://issues.apache.org/jira/browse/SLING-2743
> > Project: Sling
> > Issue Type: Bug
> > Components: JCR
> > Affects Versions: JCR Jackrabbit Server 2.1.0
> > Reporter: Ian Boston
> > Assignee: Ian Boston
> > Attachments: SLING-2743.patch
> >
> >
> > Findbugs is reporting that the initialization of the static moduleCache,
> loginModuleTracker is unsafe in a threaded environment. The method should
> probably be synchronized to ensure that only one thread accesses the method
> at a time.
> > FindBug Report:
> > Bug: Incorrect lazy initialization and update of static field
> org.apache.sling.jcr.jackrabbit.server.impl.Activator.moduleCache in
> org.apache.sling.jcr.jackrabbit.server.impl.Activator.getLoginModules()
> > This method contains an unsynchronized lazy initialization of a static
> field. After the field is set, the object stored into that location is
> further updated or accessed. The setting of the field is visible to other
> threads as soon as it is set. If the futher accesses in the method that set
> the field serve to initialize the object, then you have a very serious
> multithreading bug, unless something else prevents any other thread from
> accessing the stored object until it is fully initialized.
> > Even if you feel confident that the method is never called by multiple
> threads, it might be better to not set the static field until the value you
> are setting it to is fully populated/initialized.
> > Confidence: High, Rank: Scary (6)
> > Pattern: LI_LAZY_INIT_UPDATE_STATIC
> > Type: LI, Category: MT_CORRECTNESS (Multithreaded correctness)
>
> --
> 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
>