Tim Halloran created ACCUMULO-853:
-------------------------------------
Summary: 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
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
