Details
-
Bug
-
Status: Closed
-
Minor
-
Resolution: Not A Problem
-
2.0-beta-2
-
None
Description
I checked out the latest code from git on the master branch, and looking at LazyASTTransformation, it doesn't appear to be multi-thread safe in the case of an instance variable annotated with @Lazy.
specifically:
private void create(FieldNode fieldNode, final Expression initExpr) { final BlockStatement body = new BlockStatement(); if (fieldNode.isStatic()) { addHolderClassIdiomBody(body, fieldNode, initExpr); } else if (isVolatile(fieldNode)) { addNonThreadSafeBody(body, fieldNode, initExpr); } else { addDoubleCheckedLockingBody(body, fieldNode, initExpr); } ...
But the JMM makes DCL-locking work with volatile, i.e. it doesn't obviate the need for DCL in the case of volatile instance variables. As written now, it seems the resulting code isn't multi-thread safe in either case (the source declares the field volatile or not)
Interestingly enough, @Singleton("lazy") (SingletonASTTransformation) does this correctly, by declaring a volatile variable (named "instance", although static) and then applying the DCL idiom (see #GROOVY-4782).
So it looks like the issue is understood but somehow isn't in @Lazy.
The fix seems rather straightforward:
modify the @Lazy FieldNode to always be volatile (whether declared volatile or not in the source)
and then create becomes:
private void create(FieldNode fieldNode, final Expression initExpr) { final BlockStatement body = new BlockStatement(); if (fieldNode.isStatic()) { addHolderClassIdiomBody(body, fieldNode, initExpr); else addDoubleCheckedLockingBody(body, fieldNode, initExpr); ...