What is wrong with a Thread.sleep(1000) in a static synchronized method?
I have a static synchronized method that calls a Thread.sleep( )
I have been told that this is wrong and may cause a lockup or cause performance problems and that I should be calling .wait(1000)
instead, but I'm not sure what to call it on or why.
this isn't my code but I think it shows what I'm doing
publicstaticsynchronized MyDatabase getDB(){
dbLoader =new DatabaseLoader();//sets up a static variable
dbLoader.load();//starts a new thread, in which my DB is loaded
while ( !dbLoader.loaded() ){
try{
Thread.sleep( 1000 );// wait for a sec before next asking if finished
}catch( InterruptedException ex ){
ex.printStackTrace();
}
}
//returns DB once loaded
return dbLoader.getDB();
}
There's nothing directly wrong with it, but it will prevent other threads acquiring the class lock - but that may be what you want.
More importantly - why are you bothering to start another thread if you're going to immediately wait for it to complete? Why not just load the database in the calling thread?
> There's nothing directly wrong with it, but it will
> prevent other threads acquiring the class lock - but
> that may be what you want.
True. Although the typical case for code that looks like this would be to use wait--usually the various threads in question require the same lock, so you have to use wait in order for the waiting thread to give it up and allow the other thread to do its work. Hard to say for sure though what he's doing.
Also, if loading is all that the other thread does, and you're waiting for that thread to die, use join. But then, if that's the case, and you're only waiting for a single other thread, then you might as well just put it all in one thread, as already indicated.
jverda at 2007-7-14 22:45:55 >
As others have noted while you sleep you hold the lock and this is generally frowned upon. However if the intent is to atomically load the DB then you do NOT want to use wait() as that would release the lock and allow multiple threads to each initiate loading of the DB.
It would be cleaner if the DatabaseLoader class exported a waitUnitlLoaded() method that internally used wait() and synchronized with the DB loader thread; but you would still want to retain the outer lock to avoid concurrent attempts at loading the DB. You'd also need to make sure there is no deadlock possibility from doing that.
