[concurrency-interest] New bug pattern, way to common

Bill Pugh pugh at cs.umd.edu
Sun Mar 23 12:17:23 EDT 2008


This isn't a data race, and synchronizing on getClass() isn't always  
wrong.

But it is a bug pattern.

Most of the time you see it, the coder is doing something stupid, not  
clever. And even in the cases where the developer careful crafted the  
code and understands what he is doing, it is likely to confuse anyone  
who has to maintain that code later.

So we'll flag it in FindBugs. Somethings flagged by FindBugs aren't  
bugs, and don't need to be fixed.

Bill


On Mar 23, 2008, at 10:11 AM, Hanson Char wrote:

> I agree it's a bug pattern, but for argument sake, synchronizing on  
> getClass() isn't necessarily a bug.  Right ?  See example below that  
> synchronizes on counting the number of time the instance of a  
> specific class is constructed.  There isn't any bug there, or is  
> there ?
>
> Hanson
>
> class A {
>     private static int count;
>
>     A() {
>         synchronized(getClass()) { inc(); }
>     }
>
>     protected void inc() { count++; }
> }
>
> class B extends A {
>      private static int count;
>      @Override protected void inc() { count++; }
> }
>
> On Sat, Mar 22, 2008 at 2:50 PM, Bill Pugh <pugh at cs.umd.edu> wrote:
> Thanks for the suggestion. We found lots of occurrences of this bug  
> pattern, including 23 in JDK 1.5 and 1.6:
>
> This includes the following code in java.awt.Label (and similar code  
> in most of the AWT components):
>
>      private static final String base = "label";
>      private static int nameCounter = 0;
>      String constructComponentName() {
>         synchronized (getClass()) {
>             return base + nameCounter++;
>         }
>      }
>
> The awt Component ones have been cleaned up in JDK 1.7, but 4 remain:
>
> M M WL: Sychronization on getClass rather than class literal in  
> sun.applet.AppletPanel$9.run()  At AppletPanel.java:[line 1041]
> H M WL: Sychronization on getClass rather than class literal in new  
> sun.misc.Timer(Timeable, long)  At Timer.java:[line 166]
> H M WL: Sychronization on getClass rather than class literal in  
> sun.misc.TimerTickThread.returnToPool()  At Timer.java:[line 611]
> H M WL: Sychronization on getClass rather than class literal in  
> sun.misc.TimerTickThread.returnToPool()  At Timer.java:[line 629]
>
>
> This detector will be in 1.3.3 final (and in rc2).
>
> 	Bill
>
>
>
> On Mar 21, 2008, at 2:01 PM, Jason Mehrens wrote:
>> Another broken pattern that findbugs (1.1.3) doesn't catch is using  
>> getClass() and synchronized.  Since getClass() doesn't always  
>> return the Class object that defines the static field (because of a  
>> sub class) it is a case of synchronized on different objects.  This  
>> may not be common but it is broken.
>>
>> The following is an example, the original was from "yet another  
>> broken double checked locking attempt":
>>
>> public class NonFinalClassShouldUseClassLiteral {
>>   private static int count;
>>   public NonFinalClassShouldUseClassLiteral() {
>>     synchronized (getClass()) {
>>       count++;
>>     }
>>   }
>> }
>>
>>
>> Jason
>>
>> Test your Star IQ Play now!
>
>
> _______________________________________________
> Concurrency-interest mailing list
> Concurrency-interest at altair.cs.oswego.edu
> http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: /pipermail/attachments/20080323/5565b2f3/attachment.html 


More information about the Concurrency-interest mailing list