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 >
