Simple program - need help :/
Hi everyone,
I wanted to write a very simple program: it is supposed to count how many
threads are accessing the same array field at the same time.
Here is the code:
import java.util.*;
class MyRunnableimplements Runnable{
privatestaticint[] count=newint[10];// the real counts of threads at the same time
privatestaticint[] check=newint[10];// checking how many thread are accessing at the same time
privatestatic Random r=new Random();
private Object lock;
public MyRunnable (Object o){
lock=o;
}
publicvoid show(){
System.out.print("Counts in the array: ");
for(int i=0;i<10;i++){
System.out.print(count[i]+" ");
}
System.out.println();
}
publicvoid run(){
int x=r.nextInt(10);// random array index
check[x]++;// mark that thread is accesing by incrementing
if(check[x]>count[x])// checking if the real value of check[x] at this moment is greater then count[x]
count[x]=check[x];// if it is then we got a new number of how many threads accessed it at the same time
check[x]--;// end of thread so decrease check[x]
synchronized(lock){
lock.notify();//notify thread which counts whether all accessing threads have finished
}
}
}
class Checkerextends Thread{
privateint count;
public Checker(int count){// how many threads will access the array
this.count=count;
}
publicvoid run(){
synchronized(this){
while(count-->0){// run this thread until all the threads accessing the array finish by letting know this thread by notify()
try{
wait();
}catch(Exception e){
e.printStackTrace();
}
}
}
}
}
publicclass Bee{
privatestaticint count=30;// how many threads accessing the array there are
publicstaticvoid main(String[] args){
Checker c=new Checker(count);
MyRunnable mr=new MyRunnable(c);
c.start();
for(int i=0;i<count;i++){
new Thread(mr).start();
}
try{
c.join();
}catch(Exception e){
e.printStackTrace();
}
mr.show();// print the score after all the threads accessing the array finish
}
}
Problem: it gets stuck very often and doesn't seem to print the true results ....
PS. I have just started playing with java so this program could be probably written
in hundreds better ways ;) All comments welcome !
Cheers,
Adrian>
# 1
Well, if it is "stuck", it's not because of a deadlock. Instead, you must be waiting on the count condition (but no threads notifying).
Basically the program is rubbish. You doing lots of operations on shared state without synchronizing. This is bad. All of your access to the check and count fields should be synchronized.
Also, you need to think about program invariants. What this means is that if you are doing a test and then an update (if -x-then-y), this should alos be atomic.
check[x]++;
if (check[x] > count[x])
Oh dear. The ++ operation is not atomic, so that 2 threads doing it at the same time might leave the value only incremented by 1. Also, another thread might change the value again inbetween the increment and the test.
You want something like:
if (check[x].incrementAndGet() > count[x])
Where check is an array of AtomicInteger
# 2
Hi,
I noticed, that after declaring arrays volatile it "works better".
It is still possible to get stuck, but it happens less frequently.
Overall , it should have been declared volatile ...
And that was my intention to do some operations on shared variables
without synchronizing, because I wanted to count how many threads
try to modify the same array's field at the same time - that was the
purpose anyway (to count the maximum number of threads accessing
the same array's field at the same time).
The other thing is that ++ is not atomic - I knew that but even then,
I was expecting some more o less accurate results. Instead - it is not
behaving well (not stable) and not counting properly :/
How would you redesign it to work properly ?
Thanks,
Adrian
# 3
> And that was my intention to do some operations on shared variables
> without synchronizing, because I wanted to count how many threads
> try to modify the same array's field at the same time
But you can't! If you access in the way you have, you get corrupt and incorrect answers. You are trying to do something that is fundamentally imossible.
> The other thing is that ++ is not atomic - I knew that but even then,
> I was expecting some more o less accurate results. Instead - it is not
> behaving well (not stable) and not counting properly
No, it's doing exactly what you are telling it you and behaving perfectly. Your program is fundamentally flawed.
