Reputation: 16590
I have the following class using synchronized in an attempt to prevent access to a key until it has been generated:
public class KeyManager {
private String key;
public KeyManager() {
genKey();
}
private void genKey() {
new Thread(new Runnable() {
synchronized(KeyManager.this) {
public void run() {
key = operationThatTakesALongTime();
}
}
}).start();
}
synchronized public String getKey() {
return key;
}
}
The problem is that getKey() sometimes gets called before the inner thread and it grabs the lock first.
What I really need is a wait in getKey() that waits only if the key is null. How to do this?
Upvotes: 2
Views: 408
Reputation: 328775
I would use a CountDownLatch
instead of reimplementing the logic.
I would also probably avoid starting a new Thread from the constructor, because it can lead to subtle concurrency bugs - instead you can simply make the genKey
method public and add the relevant javadoc (this method must be called first blabla):
public class KeyManager {
private volatile boolean genStarted = false;
private final CountDownLatch keyGenerated = new CountDownLatch(1);
private String key;
public void genKey() {
genStarted = true;
new Thread(new Runnable() {
public void run() {
key = operationThatTakesALongTime();
keyGenerated.countDown();
}
}).start();
}
public String getKey() throws InterruptedException {
if (!genStarted) throw new IllegalStateException("you must run genKey first");
keyGenerated.await();
return key;
}
}
Upvotes: 2
Reputation: 14847
Mhm
synchronized public long getKey()
{
while (key == null)
{
try
{
wait();
}
catch(InterruptedException e)
{
/* handle here */
}
}
return key;
}
wait
will let the thread to release the lock, then could take it again when notify
is called. If key is still null it will release the lock again.
But i think it's not the best way to do this.
It throws InterruptedException
.
Upvotes: 3