I am sure, there are multiple answers and blogs for the same question. But, I am learning its concepts and came up with the below code, can someone please help me figure out, why it is not giving the correct output.
public class OddEventThread {
public static void main(String[] args) {
SharedResource obj = new SharedResource();
OddThread oddThread = new OddThread(obj);
EvenThread evenThread = new EvenThread(obj);
System.out.println("Starting Odd/Even Thread");
oddThread.start();
evenThread.start();
}
}
class OddThread extends Thread {
SharedResource obj;
public OddThread(SharedResource obj) {
this.obj = obj;
}
@Override
public void run() {
System.out.println("OddThread");
obj.printOdd();
}
}
class EvenThread extends Thread {
SharedResource obj;
public EvenThread(SharedResource obj) {
this.obj = obj;
}
@Override
public void run() {
System.out.println("EvenThread");
obj.printEven();
}
}
class SharedResource {
private int N = 10;
private int counter = 1;
public void printOdd() {
System.out.println("printOdd");
synchronized (this) {
System.out.println("OddThread: Counter: " counter);
while (counter <= N) {
if (counter % 2 != 0) {
System.out.println(counter);
} else {
try {
System.out.println("OddThread: Wait: Counter: " counter);
wait();
} catch (InterruptedException e) {
System.out.println("Interrupted Exception!");
}
}
counter ;
System.out.println("OddThread: Notify: Counter: " counter);
notify();
}
}
}
public void printEven() {
System.out.println("printEven");
synchronized (this) {
System.out.println("EvenThread: Counter: " counter);
while (counter <= N) {
if (counter % 2 == 0) {
System.out.println(counter);
} else {
try {
System.out.println("EvenThread: Wait: Counter: " counter);
wait();
} catch (InterruptedException e) {
System.out.println("Interrupted Exception!");
}
}
counter ;
System.out.println("EvenThread: Notify: Counter: " counter);
notify();
}
}
}
}
Output:
Starting Odd/Even Thread
OddThread
printOdd
EvenThread
printEven
OddThread: Counter: 1
1
OddThread: Notify: Counter: 2
OddThread: Wait: Counter: 2
EvenThread: Counter: 2
2
EvenThread: Notify: Counter: 3
EvenThread: Wait: Counter: 3
OddThread: Notify: Counter: 4
OddThread: Wait: Counter: 4
EvenThread: Notify: Counter: 5
EvenThread: Wait: Counter: 5
OddThread: Notify: Counter: 6
OddThread: Wait: Counter: 6
EvenThread: Notify: Counter: 7
EvenThread: Wait: Counter: 7
OddThread: Notify: Counter: 8
OddThread: Wait: Counter: 8
EvenThread: Notify: Counter: 9
EvenThread: Wait: Counter: 9
OddThread: Notify: Counter: 10
OddThread: Wait: Counter: 10
EvenThread: Notify: Counter: 11
OddThread: Notify: Counter: 12
Here is my thought process of coming up with this solution: Since, we have 2 threads to print numbers from 1 to 10. So, both this thread would share this, hence came up with one sharedObj for both the threads. Since, I have the same object shared between 2 threads, the synchronized block is used with the same lock(sharedObj lock - this).
CodePudding user response：
There are some issues in your implementation regarding its design, generalization and general logic.
You're declaring two classes which basically do the exact same thing: printing numbers. The only thing that differs is the condition: whether the printed numbers must be odd or even. You could already achieve this with one Thread class, where the only thing to parameterize is the printing condition. The printOdd() and printEven() methods are almost a copy/paste.
Furthermore, the class responsibilities are not well dealt. Your SharedObject class is basically a counter, which not only keeps track and increments the counting value, but it has also to deal with both threads' logic, which is something that should not fall on it. Its sole goal should be to increment a shared value in a consistent way with parallel execution. You should redirect that printing logic within your Thread, since, as you can see, their code basically consists in making a single call.
Answer
Finally, the logic within your printOdd and printEven() methods has some loopholes. Both threads manage to print their corresponding number type only once at the start (the System.out.println without any text). This is because:
- One of the threads acquires the SharedObject's monitor, let's say OddThread, which means that EvenThread awaits OddThread to release the lock in order to enter the synchronized block. At this point, OddThread prints its corresponding number (1):
if (counter % 2 != 0) {
System.out.println(counter);
}
- Then, OddThread increments the value to 2, prints the notify statement and finally notifies the other thread (bear in mind that while the thread is doing all of this, it's still owning the SharedObject's monitor).
counter ;
System.out.println("OddThread: Notify: Counter: " counter);
notify();
- Then, the loop ends and the
whilecondition is tested. The fowllowingiffails as the number is now even and OddThread releases the lock and waits until it gets notified by EvenThread.
try {
System.out.println("OddThread: Wait: Counter: " counter);
wait();
} catch (InterruptedException e) {
System.out.println("Interrupted Exception!");
}
- Then, EvenThread can finally enter the synchronized statement and prints its value, which is 2.
if (counter % 2 == 0) {
System.out.println(counter);
}
- Then, it increments the value from 2 to 3, prints the notify statement with value 3 (so it's printing twice and the second time with an odd value even though it's the even thread) and finally notifies the other thread.
counter ;
System.out.println("EvenThread: Notify: Counter: " counter);
notify();
- Then, EvenThread tests the
whilecondition, fails theifstatement, prints the waiting statement and then awaits for OddThread to awake it.
try {
System.out.println("EvenThread: Wait: Counter: " counter);
wait();
} catch (InterruptedException e) {
System.out.println("Interrupted Exception!");
}
- From now on, each thread will continue from their last
wait()invocation. Both of them resume when the number is their right "type", but then they increment its value, making it their opposing type, and then printing the notify statement. This, not only explains why each thread prints their opposing number type, but it also explains why they keep printing even after the max value 10 has been reached. They both increment one more time after reaching the lastwhileiteration since they both resume from their lastwait()call.
Solution
Here is an implementation with all the design, generalization and logic loopholes fixed.
class Main {
public static void main(String[] args) {
SharedCounter counter = new SharedCounter();
ThreadPrintingNums oddThread = new ThreadPrintingNums("OddThread", counter, false, 10);
ThreadPrintingNums evenThread = new ThreadPrintingNums("EvenThread",counter, true, 10);
System.out.println("Starting Threads");
oddThread.start();
evenThread.start();
}
}
class ThreadPrintingNums extends Thread {
private SharedCounter counter;
private boolean flagPrintEven;
private int max;
public ThreadPrintingNums(String threadName, SharedCounter obj, boolean flagPrintEven, int max) {
setName(threadName);
this.counter = obj;
this.flagPrintEven = flagPrintEven;
this.max = max;
}
@Override
public void run() {
while (counter.getCounter() <= max) {
if (counter.getCounter() % 2 == (flagPrintEven ? 0 : 1)) {
System.out.printf("%s => %d%n", getName(), counter.getCounter());
counter.incCounter();
} else {
try {
synchronized (counter) {
counter.wait();
}
} catch (InterruptedException e) {
System.out.printf("%s interrupted exception", getName());
System.exit(-1);
}
}
}
}
}
class SharedCounter {
private int counter;
public SharedCounter() {
this.counter = 1;
}
public synchronized int getCounter() {
return counter;
}
public synchronized void incCounter() {
counter ;
notify();
}
}