groovy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Wagenleitner <>
Subject Re: groovy git commit: Fixing squid:S2444 - Lazy initialization of "static" fields should be "synchronized"
Date Wed, 06 Jan 2016 17:50:32 GMT
It does not look like it relies on the runtime type so that I think using
HMC.class instead of getClass() would be ok, and if that is case it might
be a candidate to be made final and initialized.

    private static final MetaClass myMetaClass =

If making it lazy is important then maybe a lazy initialization holder
static nested class could be used and referenced in the getProperty(String)
method when needed.  Both should be thread-safe, the first being close to
what it was doing and the latter deferring the init to even later.  My
guess is the cost of a single call to getMetaClass may not be worth the
added complexity of making it lazily initialized.

On Wed, Jan 6, 2016 at 1:09 AM, Jochen Theodorou <> wrote:

> nono, iot is actually good to question things. Having two people review
> things is good, and I must say, I think I rushed a bit here and did misread
> the code (too much task switching and too little time for Groovy these days
> I assume).
> So what is myMetaClass for? When I did the review I assumed it is for the
> object we "handle". But that is not the case. Instead this is really the
> meta class for the handle itself, used by the handle itself.
> What does it mean then to have this static? Subclasses will not influence
> that and there won't be any per instance meta classes for this. Now I think
> it would be no good to have a per instance meta class on a handle meta
> class... I mean a meta class of a meta class... well...
> In fact I now think we should do a different change. If we decide to have
> this static, then I guess the class should actually be final.
> And then I would suggest writing a static method to get this meta class,
> which does also do the init and everything wanting to work with the
> myMetaClass should use this method then. This actually moves the init of
> the meta class to an even later point than it is right now, but that is
> fine.
> As for concurrency... I think if done this way it is no problem. Multiple
> threads might create multiple meta classes and they are different instances
> and all, but the will do the same, so in the end it does not matter which
> of those is used in the end. The meta class itself is responsible for
> proper synchronization and initializaton of anything that is visible across
> threads, so no external synchronization is required.
> what do you guys think?
> bye Jochen
> Am 06.01.2016 um 00:27 schrieb John Wagenleitner:
>> My mistake, I didn't know there was a PR associated that Jochen had
>> already reviewed.  Sorry about that.
>> On Tue, Jan 5, 2016 at 2:45 PM, Pascal Schumacher
>> < <>> wrote:
>>     Yes, I agree the change most likely reduces performance, but
>>     increases thread-safety  (prevent that different Threads may work
>>     with different Metaclasses etc.)
>>     I do not know enough about this area of the code to judge if the
>>     lack of thread-safety is really a concern.
>>     I just merged the pull request because of Jochens +1 vote.
>>     Am 05.01.2016 um 20:31 schrieb John Wagenleitner:
>>>     Not sure but wonder if HandleMetaClass#myMetaClass was static to
>>>     avoid having to perform a lookup for each HMC instance.  Probably
>>>     not a issue but thought I'd bring attention to it.
>>>     On Tue, Jan 5, 2016 at 10:13 AM, <
>>>     <>> wrote:
>>>         Repository: groovy
>>>         Updated Branches:
>>>           refs/heads/master 586a316da -> c5f17abbe
>>>         Fixing squid:S2444 - Lazy initialization of "static" fields
>>>         should be "synchronized"
>>>         <snip>
>>> ----------------------------------------------------------------------
>>>         diff --git
>>>         a/src/main/org/codehaus/groovy/runtime/
>>>         b/src/main/org/codehaus/groovy/runtime/
>>>         index f421131..9167f5c 100644
>>>         --- a/src/main/org/codehaus/groovy/runtime/
>>>         +++ b/src/main/org/codehaus/groovy/runtime/
>>>         @@ -24,7 +24,7 @@ import java.lang.reflect.Method;
>>>          public class HandleMetaClass extends DelegatingMetaClass {
>>>              private Object object;
>>>         -    private static MetaClass myMetaClass;
>>>         +    private MetaClass myMetaClass;
>>>              private static final Object NONE = new Object();
>>>              public HandleMetaClass(MetaClass mc) {
>>>         <snip>

View raw message