Reputation: 3
Does anybody know why after compiling this code, (i think) one thread still waiting for something? I want to increment a count with the threads with id 10,11 and 12 one after one, until 50. In the end, it works, but the red button (terminate) is still red, which means that the program it's still running, and probably waiting for something.
I think that when the count is 50, the return should work and exit from method. Maybe it does, but is not sufficient.
Here's the code:
public class App12 {
Semaphore sem1 = new Semaphore(1);
Semaphore sem2 = new Semaphore(0);
Semaphore sem3 = new Semaphore(0);
int count = 0;
public void inc() throws InterruptedException {
while (true) {
if (Thread.currentThread().getId() == 10) {
sem1.acquire();
if (count != 50) {
count++;
System.out.println("Thread" + Thread.currentThread().getId() + " has incremented " + count);
sem2.release();
} else
return;
}
if (Thread.currentThread().getId() == 11) {
sem2.acquire();
if (count != 50) {
count++;
System.out.println("Thread" + Thread.currentThread().getId() + " has incremented " + count);
sem3.release();
} else
return;
}
if (Thread.currentThread().getId() == 12) {
sem3.acquire();
if (count != 50) {
count++;
System.out.println("Thread" + Thread.currentThread().getId() + " has incremented " + count);
sem1.release();
} else
return;
}
}
}
public static void main(String[] args) {
App12 ap = new App12();
for (int i = 0; i < 3; i++) {
Thread th1 = new Thread(new Runnable() {
public void run() {
try {
ap.inc();
// dec3();
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
}
});
th1.start();
}
}
}
Upvotes: 0
Views: 852
Reputation: 140
Release locks to finish threads.
import java.util.concurrent.Semaphore;
public class App12 {
Semaphore sem1 = new Semaphore(1);
Semaphore sem2 = new Semaphore(0);
Semaphore sem3 = new Semaphore(0);
int count = 0;
public void inc() throws InterruptedException {
while (true) {
if (Thread.currentThread().getId() == 10) {
sem1.acquire();
if (count != 50) {
count++;
System.out.println("Thread" + Thread.currentThread().getId() + " has incremented " + count);
} else {
sem2.release();
return;
}
sem2.release();
}
if (Thread.currentThread().getId() == 11) {
sem2.acquire();
if (count != 50) {
count++;
System.out.println("Thread" + Thread.currentThread().getId() + " has incremented " + count);
} else {
sem3.release();
return;
}
sem3.release();
}
if (Thread.currentThread().getId() == 12) {
sem3.acquire();
if (count != 50) {
count++;
System.out.println("Thread" + Thread.currentThread().getId() + " has incremented " + count);
} else {
sem1.release();
return;
}
sem1.release();
}
}
}
public static void main(String[] args) {
App12 ap = new App12();
for (int i = 0; i < 3; i++) {
Thread th= new Thread(new Runnable() {
public void run() {
try {
ap.inc();
// dec3();
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
}
});
th.start();
}
}
}
Upvotes: 0
Reputation: 6456
Your problem is that you never release the locks on the threads, and they end up waiting forever.
From your logic, Thread A release B releases C.
However, once you hit 50, Thread A returns. Never releasing Thread B and C.
So what you need is an exit condition releasing all other waiting threads.
For example (in your while loop):
if(count == 50) {
sem2.release();
sem3.release();
sem1.release();
}
The issue is that once 11 or 12 increment they immidiatelly enter the loop again, at which point they lock on the semathor waiting to be released. If your count however was 50, the thread releasing it will return without ever entering your if condition.
Alternatively, you should be able to add a release on the else clause, so all threads get released.
Hope that helps,
Artur
EDIT: Here is your full code with the fix implemented:
public class App12 {
Semaphore sem1 = new Semaphore(1);
Semaphore sem2 = new Semaphore(0);
Semaphore sem3 = new Semaphore(0);
int count = 0;
public void inc() throws InterruptedException {
while (true) {
if (Thread.currentThread().getId() == 10) {
sem1.acquire();
if (count != 50) {
count++;
System.out.println("Thread " + Thread.currentThread().getId() + " has incremented " + count);
sem2.release();
} else {
sem2.release();
return;
}
}
if (Thread.currentThread().getId() == 11) {
sem2.acquire();
if (count != 50) {
count++;
System.out.println("Thread " + Thread.currentThread().getId() + " has incremented " + count);
sem3.release();
} else {
sem3.release();
return;
}
}
if (Thread.currentThread().getId() == 12) {
sem3.acquire();
if (count != 50) {
count++;
System.out.println("Thread " + Thread.currentThread().getId() + " has incremented " + count);
sem1.release();
} else {
sem1.release();
return;
}
}
}
}
public static void main(String[] args) {
App12 ap = new App12();
for (int i = 0; i < 3; i++) {
Thread th1 = new Thread(new Runnable() {
public void run() {
try {
ap.inc();
// dec3();
System.out.println("Exist ");
} catch (InterruptedException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
}
});
th1.start();
}
}
}
NOTE: I am releasing the semathors once the count is 50, so that the other threads can exit.
Upvotes: 1