You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "Tim Halloran (JIRA)" <ji...@apache.org> on 2012/11/13 23:18:12 UTC

[jira] [Updated] (ACCUMULO-853) Fields and parameters that are used as locks change to be final (where possible)

     [ https://issues.apache.org/jira/browse/ACCUMULO-853?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tim Halloran updated ACCUMULO-853:
----------------------------------

    Attachment: accumulo.patch

Patch for final locks
                
> Fields and parameters that are used as locks change to be final (where possible)
> --------------------------------------------------------------------------------
>
>                 Key: ACCUMULO-853
>                 URL: https://issues.apache.org/jira/browse/ACCUMULO-853
>             Project: Accumulo
>          Issue Type: Improvement
>            Reporter: Tim Halloran
>            Priority: Minor
>              Labels: patch
>         Attachments: accumulo.patch
>
>
> Examining locking in the code using SureLogic JSure I noticed many locks are not declared final that could be.
> One reference why this may be "bad" is CERT:
> https://www.securecoding.cert.org/confluence/display/java/LCK01-J.+Do+not+synchronize+on+objects+that+may+be+reused
> That said :-), this has two possible impacts of more immediate concern: memory model and maintainability.
> 1) The memory model only guarantees publication of final fields that are not safely published -- this is highly desired for fields used as lock objects because they may be the mechanism for safe publication. Yes it might work, but as my buddy Brian Goetz says, Just because you ran around with holding scissors all last week and didn't get hurt, is not a good rational to keep doing it.
> 2) The CERT "rule" is about mutation -- the final flag helps to clarify that the reference to the object used as a log is not intended to be changed. Avoiding a change that breaks a locking model.
> I also changed some lock objects from new String("foo") to new Object() (see patch), could be reverted (but left final) if there is some reason for the use of the larger String object. There was none I could see.
> I'm trying to verify a large portion of the lock usage statically and use a dynamic tool (SureLogic Flashlight) to check other aspects, but this was a simple patch I could provide right away as a small improvement.
> I have a patch, will attach (not a Jira expert yet)

--
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