RE: [Eap-list] BUG: Code Inspection: Singleton not recognized

Thu, 21 Mar 2002 09:24:08 -0800

Title: Сообщение
>If you'd like to play safe game you should remove condition in line 23 or make the whole method synchronized.
 
Condition in ine 23 can't be removed, becose synchronization in next line is "bottler's cone" (week place).
If we remove it the code:
 
21:  public static Singleton getInstance() {
22:    synchronized (synchronizer) {
23:      if (instance == null) {
24:        instance = new Singleton(...);
25:      } 
26:    } 
27:    return instance; 
28:  }
 
this just equive to code:
21:  public static synchronized Singleton getInstance() {
22:    if (instance == null) {
23:      instance = new Singleton(...);
24:    } 
25:    return instance; 
26:  }
 
But this code mean that for any request only one thread may use this method (always synchronized). Why? I only want to get object instance. So, i can't make method syncrhonized, becose to read created instance it not nessary. But i must make syncronize instance creation. Ok. Then i already have instance i'm do not synrhonized read instance, otherwise i do synchronized instance creation. Where I wrong?
 
>we can't support multithread logic in analyzer because it would make any inspections completely useless. Imagine we have:
>a = 5;
>if (a == 5) {}
 
 
Why? Logic are very simple. The (a == 5) may be false then:
1. variable a is global variable of class
2. variable a is not final
3. method with it conditions alocated is not synchronized (or have syncronized block by local static variable)
 
PS. I think too that thread analis is very interect. Becose java used in Internet projects as language leader. Many requested pages (JSP or servlets, singletons - it's child of Internet life) may contains any troubles with synchronization or threads collisions.
 
-----Original Message-----
From: Maxim Shafirov [mailto:[EMAIL PROTECTED]]
Sent: Thursday, March 21, 2002 6:57 PM
To: [EMAIL PROTECTED]
Subject: Re: [Eap-list] BUG: Code Inspection: Singleton not recognized

Alexey,
Unfortunately we can't support multithread logic in analyzer because it would make any inspections completely useless. Imagine we have:
a = 5;
if (a == 5) {}
According to you we can't complain a == 5 is always true because this thread could be interrupted between these thow lines of code!
Moreover, your code does not help in the situation it seems should help in. Imagine the thread would switch between after condition in line 28 succesfully checked and before assignment in line 29 is executed...
If you'd like to play safe game you should remove condition in line 23 or make the whole method synchronized.

Best regards,
Maxim Shafirov
JetBrains, Inc / IntelliJ Software
http://www.intellij.com
"Develop with pleasure!"
 
 
----- Original Message -----
Sent: Thursday, March 21, 2002 5:39 PM
Subject: [Eap-list] BUG: Code Inspection: Singleton not recognized

21:  public static Singleton getInstance() {
22:    // Check instance existing
23:    if (instance == null) {
24:      // Lock any others (we can't put this before previos if, becose it make week place in code (method invoke for get object - too many invokations hapend))

25:      // We must to be sure that only one instance (only one thread) make instance creation
26:      synchronized (synchronizer) {
27:        // Check that instance isn't already created (theoretically its may happened, fisical - maybe no, but...)
28:        if (instance == null) {
29:          instance = new FileCache();
30:        }
31:      }
32:    }
33:    return instance;
34:  }

So, we see to "if conditions": And this reasonable to Code Analiser to say:
Problem synopsis:
Condition instance == null at line 28 is always true.

But this not right.

Thanks.

PS. The Code Inspection is great idea in IDEA :) Thank you for this gem! :)

Alexey Efimov - Software Engineer
Sputnik Labs
1st Kolobovsky per., 6/3
Moscow, 103051, Russia
Phone: +7 (095) 725 5444
Direct: +7 (501) 401 3217
Fax: +7 (095) 725 5443
E-Mail: mailto:[EMAIL PROTECTED]
http://www.spklabs.com

Reply via email to