Reputation: 2535
Hi I have a situation in which I have to allow only one thread to say update a variable.
There is a trigger, which might invoke multiple threads to update this variable, however the update should happen only once by the first thread whichever arrives at the critical section.
Ideally the flow should be like follows:
Thread-1; Thread-2 and Thread-3 are invoked to update a variable in critical section guarded by a lock or a mutex
Critical section using this guard allows only one thread to enter, Thread-2 and Thread-3 wait just outside.
Once this variable is updated by Thread-1; Thread-2 and Thread-3 resume with other work without causing an effect on the variable.
I have come up with the following implementation, but I am not able to make other threads wait and skip updation:
public class Main {
private static ReentrantLock lock = new ReentrantLock();
private int counter = 0;
public static void main(String[] args) {
Main m = new Main();
new Thread(m::doSomeOperation).start();
new Thread(m::doSomeOperation).start();
new Thread(m::doSomeOperation).start();
}
private void doSomeOperation() {
try {
System.out.println("Thread about to acquire lock: " + Thread.currentThread().getName());
if (lock.tryLock()) {
System.out.println("Lock held by " + Thread.currentThread().getName() + " " + lock.isHeldByCurrentThread());
counter++;
// Thread.sleep(3000);
System.out.println("Counter value: " + counter + " worked by thread " + Thread.currentThread().getName());
}
} catch (Exception ex) {
ex.printStackTrace();
} finally {
if (lock.isHeldByCurrentThread()) {
lock.unlock();
System.out.println("Unlocked: " + Thread.currentThread().getName());
}
}
}
}
At the end, the counter value is 3, I want counter value to be one also I want other threads to wait till the first thread updates the counter. Ideas appreciated. I would prefer a solution using locks / mutex rather than wait and notify.
Output:
Thread about to acquire lock: Thread-0
Lock held by Thread-0 true
Thread about to acquire lock: Thread-1
Counter value: 1 worked by thread Thread-0
Unlocked: Thread-0
Thread about to acquire lock: Thread-2
Lock held by Thread-2 true
Counter value: 2 worked by thread Thread-2
Unlocked: Thread-2
Process finished with exit code 0
Note My use case, is different - the example of updating a counter is for simplicity sake. Actually I am updating a session token in the doSomeOperation method.
Upvotes: 2
Views: 1983
Reputation: 11999
The Java concurrency utils don't seem to offer something exactly like what you (and me) need. But here's a relatively simple approach.
public class Sample {
private String token;
public String getToken(boolean forceNew) {
boolean expired = false; // would be replace with actual check
if (expired) {
return getNewAccessToken();
} else {
return token;
}
}
private String getNewAccessToken() {
String currentToken = token; // Get current token
synchronized (currentToken) { // Semaphore on the current token
if (currentToken != token) // Now check if it changed after obtaining the lock
return token; // If so, it has just been renewed and we don't need another exchange
// Otherwise do the renew
Response response = null;
// Token fetch/refresh code here...
// response = result of some call
processResponse(response);
return token;
}
}
private void processResponse(Response response) {
token = "test"; // in reality get the value from the response
}
}
Suppose thread A and B arrive at getNewAccessToken()
simultaneously. Following conditions are possible:
token
field to local variable currentToken
. Thread B does the same in its invocation. Now thread A enters the synchronized block. Thread B has to wait. Thread A renews the token and exits the synchronized block. Thread B can now enter but the token
field is no longer the same object as the currentToken
variable, so it just returns.token
field to local variable currentToken
and enters the synchronized block. Only now does thread B assign the token
field to local variable currentToken
. Thread B has to wait until A exits the synchronized block before it can proceed. There's two possiblities here:
processResponse
before thread B assigned token
to currentToken
. Thread B will then do an unnecessary refresh because currentToken == token. In fact it won't have to wait for the synchronization because A and B are locked onto a different object (A on the old token, B on the new one).The situation is not 100% waterproof because there are still situations where an unnecessary renewal is done for concurrent threads. But this would not occur during the part of the process that takes the longest, the actual renewal of the token via some network call. After that there isn't really any difference between the scenario where we just happen to hit the narrow window between thread A having finished the network call and updating the field while thread B enters, of thread B just coming in after A is entirely done.
If you want to refine this to avoid unnecessary updates you could also look at some expiration info from the response or keep a timestamp of the last renewal and block new ones within too short a time frame.
Upvotes: 0
Reputation: 1038
Problem is occurring because Initially one thread increases the counter and release the lock and your program is running so fast that, once first thread releases the lock then another thread enters the method and it sees the lock free so it acquires the lock and increase the counter further. You can make use of countDownLatch
in this case.
Here one thread which get the lock decrease the latch count by one and makes it zero, after that none of the thread will be able to process because
latch.getCount()==1
condition will fail.
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.locks.ReentrantLock;
public class Test2 {
private static ReentrantLock lock = new ReentrantLock();
private static CountDownLatch latch= new CountDownLatch(1);
private int counter = 0;
public static void main(String[] args) {
Test2 m = new Test2();
new Thread(m::doSomeOperation).start();
new Thread(m::doSomeOperation).start();
new Thread(m::doSomeOperation).start();
}
private void doSomeOperation() {
try {
System.out.println("Thread about to acquire lock: " + Thread.currentThread().getName());
if (lock.tryLock() && latch.getCount()==1) {
System.out.println("Lock held by " + Thread.currentThread().getName() + " " + lock.isHeldByCurrentThread());
counter++;
latch.countDown();
Thread.sleep(3000);
System.out.println("Counter value: " + counter + " worked by thread " + Thread.currentThread().getName());
}
System.out.println("Exiting" + Thread.currentThread().getName());
} catch (Exception ex) {
ex.printStackTrace();
} finally {
if (lock.isHeldByCurrentThread()) {
lock.unlock();
System.out.println("Unlocked: " + Thread.currentThread().getName());
}
}
}
}
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.locks.ReentrantLock;
public class Test2 {
private static ReentrantLock lock = new ReentrantLock();
private static CountDownLatch latch= new CountDownLatch(1);
private int counter = 0;
public static void main(String[] args) {
Test2 m = new Test2();
new Thread(m::doSomeOperation).start();
new Thread(m::doSomeOperation).start();
new Thread(m::doSomeOperation).start();
}
private void doSomeOperation() {
try {
System.out.println("Thread about to acquire lock: " + Thread.currentThread().getName());
if (lock.tryLock() && latch.getCount()==1) {
System.out.println("Lock held by " + Thread.currentThread().getName() + " " + lock.isHeldByCurrentThread());
counter++;
latch.countDown();
Thread.sleep(3000);
System.out.println("Counter value: " + counter + " worked by thread " + Thread.currentThread().getName());
}
System.out.println("Exiting" + Thread.currentThread().getName());
} catch (Exception ex) {
ex.printStackTrace();
} finally {
if (lock.isHeldByCurrentThread()) {
lock.unlock();
System.out.println("Unlocked: " + Thread.currentThread().getName());
}
}
}
}
Upvotes: 1
Reputation: 521194
You could use AtomicInteger
here and forgo with worrying about formal locks:
public class Worker {
private static AtomicInteger counter = new AtomicInteger(0);
private void doSomeOperation() {
counter.incrementAndGet();
System.out.println("Counter value: " + counter + " worked by thread " + Thread.currentThread().getName());
}
}
public class Main {
public static void main(String[] args) {
Worker w = new Worker();
new Thread(w::doSomeOperation).start();
new Thread(w::doSomeOperation).start();
new Thread(w::doSomeOperation).start();
}
}
Upvotes: 1
Reputation: 1454
If you know the exact number of threads you starts each round: You can do it like this:
private int threadCounter;
private int threadCount = 3;
private void doSomeOperation() {
try {
System.out.println("Thread about to acquire lock: " + Thread.currentThread().getName());
if (lock.tryLock()) {
System.out.println("Lock held by " + Thread.currentThread().getName() + " " + lock.isHeldByCurrentThread());
if (threadCounter++ % threadCount == 0) {
counter++;
// Thread.sleep(3000);
System.out.println("Counter value: " + counter + " worked by thread " + Thread.currentThread().getName());
}
}
} catch (Exception ex) {
ex.printStackTrace();
} finally {
if (lock.isHeldByCurrentThread()) {
lock.unlock();
System.out.println("Unlocked: " + Thread.currentThread().getName());
}
}
}
It will allow to increment counter only to the the first thread in each round.
Upvotes: 0