I have 3 class like this:
Source.java
public class Source extends Thread{
private int x= 0;
public void increment(int id){
x ;
System.out.println(id " " x);
}
}
Task.java
public class Task extends Thread{
private Source source;
private int id;
public Task(Source source, int id){
this.source=source;
this.id=id;
}
public void run(){
for (int i=0;i<100;i ){
try{Thread.sleep(1000);}catch(InterruptedException e){}
source.inc(id);
}
}
}
Main.java
public class Main{
public static void main(String[] args) throws IOException{
Source source = new Source();
Task t1=new Task(source,1);
Task t2=new Task(source,2);
t1.start();
t2.start();
}
}
i want when the x of the class Source will be equal to 4 only one task continues to increment x until x is equal to 8, we return to normal. The result will look like this:
1 1
2 2
1 3
2 4
1 5
1 6
1 7
1 8
1 9
1 10
2 11
1 12
2 13
...
How do I modify the code to achieve the desired result?
CodePudding user response:
Basically you have two threads that modify the same variable x. There is no garantee about the order of execution. You should synchronize.
With your current implementation you may face a problem (The race condition problem): Race condition example
Something like this is an scenario that most likely is going to happen to you:
....
1 3
2 4
2 5
1 6
1 7
2 7
1 8
2 9
1 10
2 10
...
As you can see the thread 2 (source 2) tries to increment the variable x when the variable has been already incremented but the value it has to increment is the old one.
x = 0
- Thread 1 reads the variable x (0)
- Thread 2 reads the variable x (0)
- Thread 1 increments variable x 1 (0 1) = 1
- Thread 2 increments variable x 1 (0 1) = 1
In order to solve this you need to synchronize your variable, an AtomicInteger would be enough. I don't think you need the extends Thread on your Source class, you can get rid of it
CodePudding user response:
Another answer would be found at https://www.baeldung.com/java-synchronized
CodePudding user response:
There are a few things off within your code. As it has been pointed out in the comments and in @Vml11's answer, your shared resource Source
does not need to extend the Thread
class and you definitely need to implement a synchronized approach to access and modify x from two different threads in order to not cause Thread Interference.
https://docs.oracle.com/javase/tutorial/essential/concurrency/interfere.html
In fact, right now both threads can access Source
not in exclusive way. This means that the changes performed by a thread can be overlapped and lost by the operations executed by the other one, since there is no happens-before
relationship between the two threads, e.i. the changes made by a thread might not be visible by the other.
https://docs.oracle.com/javase/tutorial/essential/concurrency/memconsist.html
In order to establish a happens-before
relationship between the threads, you need to implement a synchronized access to Source
, for example by adding the synchronized
keyword to the increment
method. Your fixed code would look like this (just for conciseness I've reduced the number of iterations from 100 to 10).
public class Main {
public static void main(String[] args) {
Source source = new Source();
Task t1 = new Task(source, 1);
Task t2 = new Task(source, 2);
t1.start();
t2.start();
}
}
class Source {
private int x = 0;
//Field to establish by the thread's id who can update x once it reaches 4
private int idThreadExclusiveInc = -1;
public synchronized void increment(int id) {
//The current thread can update x only if the increment operation isn't exclusive to a thread or if the current thread can update x
if (idThreadExclusiveInc == -1 || idThreadExclusiveInc == id) {
x ;
//If x is equal to 4 then the current thread sets its id to exclusively increment x until it reaches 8
idThreadExclusiveInc = x == 4 ? id : idThreadExclusiveInc;
//If x is equal to 8 the var holding the thread's id for exclusive increment is reset
idThreadExclusiveInc = x == 8 ? -1 : idThreadExclusiveInc;
System.out.printf("id: %d - x: %d%n", id, x);
}
}
}
class Task extends Thread {
private Source source;
private int id;
public Task(Source source, int id) {
this.source = source;
this.id = id;
}
public void run() {
for (int i = 0; i < 10; i ) {
try {
Thread.sleep(500);
} catch (InterruptedException e) {
}
source.increment(id);
}
}
}
Here is a link to test the code above: