Return-Path: X-Original-To: apmail-groovy-dev-archive@minotaur.apache.org Delivered-To: apmail-groovy-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 09DD418FCC for ; Wed, 6 Jan 2016 07:51:06 +0000 (UTC) Received: (qmail 62319 invoked by uid 500); 6 Jan 2016 07:51:05 -0000 Delivered-To: apmail-groovy-dev-archive@groovy.apache.org Received: (qmail 62277 invoked by uid 500); 6 Jan 2016 07:51:05 -0000 Mailing-List: contact dev-help@groovy.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@groovy.apache.org Delivered-To: mailing list dev@groovy.apache.org Received: (qmail 62267 invoked by uid 99); 6 Jan 2016 07:51:05 -0000 Received: from Unknown (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 06 Jan 2016 07:51:05 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 06CA7C0061 for ; Wed, 6 Jan 2016 07:51:05 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.997 X-Spam-Level: ** X-Spam-Status: No, score=2.997 tagged_above=-999 required=6.31 tests=[HTML_MESSAGE=3, RCVD_IN_MSPIKE_H2=-0.001, RP_MATCHES_RCVD=-0.001, SPF_PASS=-0.001] autolearn=disabled Received: from mx1-us-west.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id bgeaS0DnFJoW for ; Wed, 6 Jan 2016 07:51:04 +0000 (UTC) Received: from mout.gmx.net (mout.gmx.net [212.227.15.15]) by mx1-us-west.apache.org (ASF Mail Server at mx1-us-west.apache.org) with ESMTPS id 77ECA20D16 for ; Wed, 6 Jan 2016 07:51:03 +0000 (UTC) Received: from [10.132.130.145] ([80.187.96.235]) by mail.gmx.com (mrgmx003) with ESMTPSA (Nemesis) id 0MXIcX-1akfms3v6r-00WJ5u for ; Wed, 06 Jan 2016 08:50:56 +0100 Subject: Re: groovy git commit: Fixing squid:S2444 - Lazy initialization of "static" fields should be "synchronized" To: dev@groovy.apache.org References: <568C4794.3000500@gmx.net> From: Pascal Schumacher Message-ID: <568CC761.1030803@gmx.net> Date: Wed, 6 Jan 2016 08:50:57 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/alternative; boundary="------------050707010309020703050109" X-Provags-ID: V03:K0:oB1ZKHfM2aZsln3V0dUD0KtFpszLhObTvDShs5sgKKV3rypBsyq tkxGAy620aoYsm4q5RDQM+UCZNlABFyYWiMWRx7TdPSq7OMxsjvUi5tYXxKOJ6SeK8KC8+k VyDJW9101PFvDs34q9ddSO6ew4rNMxVeoQmVPgCe8Ni/ftBh2N+NY64SkM/x+6mdSThzbRp fw8K9vdmS1WtUAAWDw6SA== X-UI-Out-Filterresults: notjunk:1;V01:K0:1ejHexpvyzw=:MchklslVrRJxwiJebpy/c3 j8/DTU0XIVwo5gQ0NHhcat1dtHoWwXqbvMLULZVem5AUYiFyJBtgVoU7Uz+XzYYUy5UTxw9nk VT+2v0cqKoGWr/YRRs/KMXk+UX38kJIejXrQHg7xm8rvBtINATiculvULx0O5UieGSpcwcv7W lD/5Gzdy0+W8XEvp6EbU0WA693Cv6HjKoPbIIiiOv9AC36z2peX82fU+RVtrsjztLfr29E6Kr BBwgLNT+cWVMHcaOr9sQydQzsjzqSmENhRKqmBFetFdXcoqPAU5rQZZShrx+Oj3EGuyEhwbFc uJRlDsiGMTykRUjOqoPzYIdChCGeAcdMpPusz6/3pMUhGzmaKcVfVSmNY8+/zCc6aNvuu4D6P 4i2AX/DbskMk2bJSMgWfUtjImn0T2lN7PLvCiYYDNMGeyciSKzlop2g5HCQfaT3LDlCn9mnfq LVBAhzmwhZnlElLCveLSBz2qQz6iI44y0DQYADrjVqgY7dHaZDvkJ/PZDQB7si4CgFXZlgTM8 6BqeO7ePk+boUQPCEglkPHlMCPcHDcz43HHbnp2s+qgz7JDVBWFOoxCtOTR/8YjkJtKElPLTX wbvanaVC2hD6m7kCQFNU/+GBUsbnbdTfUmwHD0wLTDSSMrABSIUnNhdxz4FbUEoulZAd8d0/S yIkt7rjdPG6CK2dFSJrEhp+JuL3cma6m46hKkzfCIC+5ITtUt2g51WHlGDJ+KViaZCJ7qTryK kx+cg4OQltBvmVMBHZIvcy2df88uI1WQ+x0RKXlcFg8HIW4878vIzz+PRF7aNjWqppCvpcIhA r1MM3FeGZ9uOrCZaxsi4OR8bFq0rw== This is a multi-part message in MIME format. --------------050707010309020703050109 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit No need to apologize. It's good that you review changes and point out potential problems. :) Thanks! 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" >> >> >> >> http://git-wip-us.apache.org/repos/asf/groovy/blob/c5f17abb/src/main/org/codehaus/groovy/runtime/HandleMetaClass.java >> ---------------------------------------------------------------------- >> diff --git >> a/src/main/org/codehaus/groovy/runtime/HandleMetaClass.java >> b/src/main/org/codehaus/groovy/runtime/HandleMetaClass.java >> index f421131..9167f5c 100644 >> --- a/src/main/org/codehaus/groovy/runtime/HandleMetaClass.java >> +++ b/src/main/org/codehaus/groovy/runtime/HandleMetaClass.java >> @@ -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) { >> >> >> > > --------------050707010309020703050109 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit
No need to apologize.

It's good that you review changes and point out potential problems. :)

Thanks!

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 <pascalschumacher@gmx.net> 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, <pascalschumacher@apache.org> wrote:
Repository: groovy
Updated Branches:
  refs/heads/master 586a316da -> c5f17abbe


Fixing squid:S2444 - Lazy initialization of "static" fields should be "synchronized"

<snip>

http://git-wip-us.apache.org/repos/asf/groovy/blob/c5f17abb/src/main/org/codehaus/groovy/runtime/HandleMetaClass.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/runtime/HandleMetaClass.java b/src/main/org/codehaus/groovy/runtime/HandleMetaClass.java
index f421131..9167f5c 100644
--- a/src/main/org/codehaus/groovy/runtime/HandleMetaClass.java
+++ b/src/main/org/codehaus/groovy/runtime/HandleMetaClass.java
@@ -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>



--------------050707010309020703050109--