[java] multithreaded singleton

[reddog33hummer]

ik was een beetje aan het proggen met java en toen gebeurde dit.

ik had een singleton die plaatjes cached die later in de AWT gebruikt worden.

voor de leken onder ons: dat betekend dat van die singleton maar 1 aangemaakt wordt en die gebruikt wordt voor alle data
(Design patterns door "de grote 4", gamma, helm,johnsen en vlissides) {nee, ik ben geen fan van hun, zo heet dat boek nu eenmaal |:(}

Om die cache te testen had ik ff een progje geschreven die met meerdere threads tergelijkertijd de cache teste. Dat gaf aleen vreemde resultaten. Na ongeveer 2 uur wist ik de fout.

code:

private static Cache instance = null;

private Cache(){
// hier komt de install van de cache maar dat is later
}

public static Cache getInstance(){
   if(instance == null){
     instance = new Cache();
   }
   return instance;
}

wat dus gebeurt is dat thread 1 na

code:

instance == null komt

op dat moment is er een task switch en dan komt thread 2 bij die controlle op dat moment worden er dus 2 cache aangemaakt.

nu had ik die getInstance synchronized gemaakt dat werkt voor de IBM java maar die van sun....

volgens de java standaard kan dit uberhoupt niet eens omdat een static functie geen object heeft en daarom met synchronized niet op slot kan.

iemand een idee om dit op te lossen zonder teveel preformance verlies of geheugengebruik ?

edit:

\\ vervangen door // in [code]
stom had ik moeten weten

[Verwijderd]

Moet je softwarematig oplossen dan ("test en zet" ofzo). Creer dan een of andere Mutex ?

[reddog33hummer]

mutexen kan je niet gebruiken hiervoor, je zit in een static functie dus geen gezamelijke mutex space

[Jelmer]

Dit zou je eigenlijk ook heel anders op moeten lossen.

Je (zij) maakt een object als ie niet bestaat in een get methode... niet echt netjes als je het mij vraagt...

Misschien is het een idee om die instance zelf te maken voordat je die thread's strart (bijv id constructor van dat object) en dat die getInstance alleen iets laten returnen (wat eigenlijk ook de bedoeling is van een 'get' methode).

[The - DDD]

Jelmer Barhorst: Dit is juist de essentie van het singleton pattern.

En wat loopt iedereen vaag te lullen, gewoon thread safe maken met wat synchonized blokken.

Synchonizeren op je variabele instance...

code:

private static Cache instance = null;

private Cache(){
// hier komt de install van de cache maar dat is later
}

public static Cache getInstance(){
  synchronized (instance)
  {
   if(instance == null){
     instance = new Cache();
   }
  }
   return instance;
}

of:
De hele metode toegankelijk maken voor 1 thread tegelijk.

code:

private static Cache instance = null;

private Cache(){
// hier komt de install van de cache maar dat is later
}

public synchronized static Cache getInstance(){
   if(instance == null){
     instance = new Cache();
   }
   return instance;
}

of:
De hele class voor andere threads dichtgooien als een thread toegang heeft tot 1 blok.

code:

private static Cache instance = null;

private Cache(){
// hier komt de install van de cache maar dat is later
}

public synchronized static Cache getInstance(){
 synchonized (this)
 {
   if(instance == null){
     instance = new Cache();
   }
 }
   return instance;
}

Elke manier heeft voor en nadelen. Zoek wat info op over synchonized. Vergeet niet dat op andere plekken waar instance aangeroepen wordt en er geen synchronizatie op staat, dat dan een thread nog steeds de variabele aan kan passen.

[mbravenboer]

The - DDD: En wat loopt iedereen vaag te lullen, gewoon thread safe maken met wat synchonized blokken.

Snif, komt er een keer iemand met een design pattern, wordt er niets voor mij overgelaten

Overigens vraag ik me wel af wat er gebeurd als je synchronized over een variabele die null kan zijn.... Ik vind het sowieso vaak wel netjes en duidelijk om even een apart lock object te maken.

[mbravenboer]

The - DDD: of: De hele class voor andere threads dichtgooien als een thread toegang heeft tot 1 blok.

public synchronized static Cache getInstance()
{
synchonized (this)

Hier is alleen niet echt een this.... toch? Die dubbele synchronized constructie vind ik sowieso wel vrij vaag. Wat probeer je precies te bereiken?

[mbravenboer]

Hier trouwens een artikel wat je erg leuk zal vinden:

http://www.javaworld.com/javaworld/jw-01-2001/jw-0112-singleton.html

Het behandelt precies alle problemen met multi-threading. Listing 2 geeft een goede manier voor het werken met lazy singletons.

[reddog33hummer]

Op dinsdag 13 november 2001 00:30 schreef The - DDD het volgende:

public synchronized static Cache getInstance(){
synchonized (this)
{
if(instance == null){
instance = new Cache();
}
}
return instance;
}
[/code]

das echt dubbele onzin
1. in een static functie heb je geen object dus mag je ook geen This gebruiken
2. Zoals ik al zij met synchronized lock je het object die je met een static niet hebt

[wasigh]

waarom die je niet iets in de trand van :

code:

class Cache
{

static
{
private static Cache instance = new Cache();
  }
}

* wasigh is er niet helemaal zeker van of dit werkt en of dit synchronized is al ga ik er vanui van wel...

maar dat zal je even moeten testen (p.s. weet ook niet zeker of de syntax correct is, gebruik dit nooit)

[Verwijderd]

Tja, nu schiet je volgens mij je doel voorbij.
Als je static methodes gebruikt heb je in principe geen threadsafe code nodig.
Waarom het ontwerp niet anders zodat je 1 thread aanmaakt?

[Dash2in1]

Op dinsdag 13 november 2001 00:30 schreef The - DDD het volgende:

(...)

code:

public synchronized static Cache getInstance(){
 synchonized (this)
 {
   if(instance == null){
 instance = new Cache();
   }
 }
   return instance;
}

Even vraag hierover, mocht dit al kunnen, zou het dan niet inhouden dat vanwege de eerste synchronized hij bij de "synchronized(this)" blijft wachten tot hij klaar is met de functie ?!

[mbravenboer]

wasigh: waarom die je niet iets in de trand van

Omdat deze oplossing niet lazy is: bij de moeilijkere varianten wordt er pas een instantie gemaakt als die ook echt nodig is. Dat kan een voordeel zijn.

* wasigh is er niet helemaal zeker van of dit werkt en of dit synchronized is al ga ik er vanui van wel...

Het static blok van een klasse wordt inderdaad alleen maar uitgevoerd als de klasse wordt geladen, dus gaat dit goed.

[wasigh]

Op dinsdag 13 november 2001 14:50 schreef mbravenboer het volgende:

[..]

Omdat deze oplossing niet lazy is: bij de moeilijkere varianten wordt er pas een instantie gemaakt als die ook echt nodig is. Dat kan een voordeel zijn.
[..]

Dat was ook het enige tegenargument wat ik kon verzinnen

[Alarmnummer]

Ik kom dit probleem zelf ook wel eens tegen en je zou er ook voor kunnen kiezen om meteen een Cache object te creeeren aan het begin van je app en hoef je verder niet meer te controleren of de singleton al is aangemaakt. Is ook een stuk sneller dan al die synchronizaties.

code:

public class Cache
{
    private static Cache instance = null;
    
    public static Cache getInstance()
    {
        return instance;
    }
    
    public static void createInstance()
    {
        if(instance!=null)
            throw new RuntimeException("Cache already is created");
            
        instance = new Cache();     
    }
    
    private Cache()
    {
        // hier komt de install van de cache maar dat is later
    }   
}

[Alarmnummer]

Op dinsdag 13 november 2001 12:50 schreef wasigh het volgende:
waarom die je niet iets in de trand van :

code:

class Cache
{

static
{
private static Cache instance = new Cache();
  }
}

* wasigh is er niet helemaal zeker van of dit werkt en of dit synchronized is al ga ik er vanui van wel...

maar dat zal je even moeten testen (p.s. weet ook niet zeker of de syntax correct is, gebruik dit nooit)

Iemand is me al voor geweest En ik zie dat hij niet meteen het object aan het begin wil creeeren.