Java Newbie struggling with threads

[Bob Dubery]

I wonder if some kind person can deconfuse me here. I have objects
running as threads interfering with each other.

Here's my class...

class primeSeek implements Runnable{
private static int found = 0;
private static int[] possibles;
public primeSeek(int[] passedarr){
possibles = passedarr;
}
public void run(){
int howMany = possibles.length;
for(int testLoop=0; testLoop<howMany;testLoop++){
int aCandidate = possibles[testLoop];
int limit = aCandidate/2;
boolean isPrime = true;
for(int aFactor = 2; aFactor <= limit; aFactor++){
if((aCandidate%aFactor) == 0){
/* aFactor is a factor ofaCandidate
and aFactor < aCandidate
and aFactor > 1
thus aCandidate is not prime */
aFactor = limit;
isPrime = false;
}
}
// was our number prime?
if(isPrime){
found ++;
}
}
// how many primes?
System.out.println(found + " Prime numbers found");
}
}



Now the code that sets up the threads (and passes each one a different
array)

// create threads to check for primes
int[] Candidates = new int[100];
for(int i=0;i<100;i++){
Candidates = LotsOfInts;
}
Thread T1 = new Thread(new primeSeek(Candidates));
T1.start();


// for(int i = 0; i < 100000000; i++){
// }
for(int i=0;i<100;i++){
Candidates = LotsOfInts[i+100];
}
Thread T2 = new Thread(new primeSeek(Candidates));
T2.start();


My problem is that T1 and T2 end up checking the same array for prime
numbers, despite my taking care to set up Candidates[] differently for
each thread. I'm guessing this is something to do with the way I have
declared possibles in primeSeek.

See that loop between the creation of the threads? If I remove the
comments then the interference goes away because T1 halts (and I
presume is garbage collected) prior to T2 being created. But this is
an inelegant way of preventing the interference.

What can I do to stop the threads interfering with each other?

Thanks in advance

Bob

 

[ALR]

Hi bob

Your thread T1 and T2 are checking the same array because you are passing
the same array to both of them.
Objects are shared between threads.
Create a new array for T2 and everything will be ok.
See the added line below (array Candidate2):

Regards
Alain

....
// fist array for T1

// create threads to check for primes
int[] Candidates = new int[100];
for(int i=0;i<100;i++){
Candidates = LotsOfInts;
}
Thread T1 = new Thread(new primeSeek(Candidates));
T1.start();

// Create a new array for T2
int [] Candidates2 = new int[100];

// for(int i = 0; i < 100000000; i++){
// }
for(int i=0;i<100;i++){

initialize the 2nd array

Candidates2 = LotsOfInts[i+100];
}

and pass it to T2

Thread T2 = new Thread(new primeSeek(Candidates2));
T2.start();

....

 

[Sergio]

Hi bob

Your thread T1 and T2 are checking the same array because you are passing
the same array to both of them.
Objects are shared between threads.
Create a new array for T2 and everything will be ok.
See the added line below (array Candidate2):

Regards
Alain

I'd say it's because he has declared "possibles" as static... so every
instance of primeSeek share this int array....

but I'm not an expert...

Sergio

 

[John C. Bollinger]

I wonder if some kind person can deconfuse me here. I have objects
running as threads interfering with each other.

Here's my class...

class primeSeek implements Runnable{
private static int found = 0;
private static int[] possibles;

-------------^^^^^^

This (making these two variables static) is your [first] problem in
combination with setting them in your constructor. Static class members
are scoped to the class, not to class instances, which is to say that
there is only one of any static member, shared by all instances of the
class. That makes static members particularly dangerous in
multithreading scenarios.

public primeSeek(int[] passedarr){
possibles = passedarr;

Here you assign the constructor argument to the static possibles
variable. The class can work with that as long as there is only one
instance, but as soon as you create another one you assign a new value
to possibles. Make possibles non-static to fix this problem. In
general, if you find yourself initializing a static variable in a
constructor then it's a good sign that something is wrong with your
design, most likely that you want an instance variable instead of a
static one.

}
[...]

Now the code that sets up the threads (and passes each one a different
array)

// create threads to check for primes
int[] Candidates = new int[100];
for(int i=0;i<100;i++){
Candidates = LotsOfInts;
}
Thread T1 = new Thread(new primeSeek(Candidates));
T1.start();


// for(int i = 0; i < 100000000; i++){
// }
for(int i=0;i<100;i++){
Candidates = LotsOfInts[i+100];
}
Thread T2 = new Thread(new primeSeek(Candidates));
T2.start();

There is a second problem here. You only ever create one array to begin
with, but you modify it while thread T1 is working on it and then tell
T2 to work on it. You must create a different array for the second
thread to work on. For example:

int[] candidates = new int[100];
// initialize candidates here
Thread t1 = new Thread(new PrimeSeek(candidates));
t1.start();

candidates = new int[100]; // THIS IS THE CRUCIAL LINE
// initialize new array now referenced by candidates
Thread t2 = new Thread(new PrimeSeek(candidates));
t2.start();

Side note: the very widely used Java naming conventions would have you
give classes names with initial capital letters, and instance variables,
local variables, and method arguments names that start with lower-case
letters. (See example above.) Others will have an easier time reading
your code if your adhere to this convention.


John Bollinger
(e-mail address removed)

 

[Bob Dubery]

Side note: the very widely used Java naming conventions would have you
give classes names with initial capital letters, and instance variables,
local variables, and method arguments names that start with lower-case
letters. (See example above.) Others will have an easier time reading
your code if your adhere to this convention.

Many thanks for that helpful tip.

 

[Bob Dubery]

Bob Dubery wrote:

// create threads to check for primes
int[] Candidates = new int[100];
for(int i=0;i<100;i++){
Candidates = LotsOfInts;
}
Thread T1 = new Thread(new primeSeek(Candidates));
T1.start();


// for(int i = 0; i < 100000000; i++){
// }
for(int i=0;i<100;i++){
Candidates = LotsOfInts[i+100];
}
Thread T2 = new Thread(new primeSeek(Candidates));
T2.start();

There is a second problem here. You only ever create one array to begin
with, but you modify it while thread T1 is working on it and then tell
T2 to work on it.

Clang!

That was the sound of a big penny dropping.

Thanks very much... this was what was causing me the grief. I'd change
the variables in the threaded object away from static, but that by
itself didn't do the trick.

Thanks too to the other kind folks who replied. Much appreciated.

 

[Bob Dubery]

There is a second problem here. You only ever create one array to begin
with, but you modify it while thread T1 is working on it and then tell
T2 to work on it. You must create a different array for the second
thread to work on.

This is quite interesting. I'd assumed (clearly incorrectly) that once
T1 had started then it would run in isolation and you couldn't effect
what was already in it's memory IE changing the array to fire up a new
thread would not effect the thread already running.

Clearly there's a reason for things working this way.

 

[alr]

Hi Sergio
You are perfectly right, this is another reason for the bug

 

[John C. Bollinger]

This is quite interesting. I'd assumed (clearly incorrectly) that once
T1 had started then it would run in isolation and you couldn't effect
what was already in it's memory IE changing the array to fire up a new
thread would not effect the thread already running.

Clearly there's a reason for things working this way.

It is the Java model, and it is generally a good one. Among other
things, it makes it considerably easier for threads to synchronize and
communicate with each other. Moreover it is easier, less complicated,
and therefore less error-prone to design a VM for that model than for
one that behaves as you had expected. Also, although your model can be
emulated within Java's model, the reverse is not the case.

Out of curiosity, what reasons do advance for it being advantageous if
it worked the other way?


John Bollinger
(e-mail address removed)

 

[Bob Dubery]

It is the Java model, and it is generally a good one. Among other
things, it makes it considerably easier for threads to synchronize and
communicate with each other. Moreover it is easier, less complicated,
and therefore less error-prone to design a VM for that model than for
one that behaves as you had expected. Also, although your model can be
emulated within Java's model, the reverse is not the case.

Out of curiosity, what reasons do advance for it being advantageous if
it worked the other way?

I'm not saying that it would be advantageous, just that it caught me
by surprise (some say that's not particularly difficult).

Once I'd experimented with what you'd pointed out earlier I was able
to create an array of 10 thread objects, fire them off in a loop and
have them run happily with no clashes. Then I started to work on
getting the number of prime numbers detected by ALL threads back into
the main thread (the initial thread that spawned the other 10).
Eventually I realised that the only way to do it was to have a common
object the threads could report to and which could also report back to
any other thread. Bingo! That worked well - once I spelled synchronize
with a 'ze' rather than an 'se' - and then the penny dropped all the
way. There was no other way for it to work.

Coming back to the original problem that was really solved (for me) by
re-initialising the passed array. Did that really happen because
arrays are passed by reference?

Thanks for the help and for lending some clarity.

Bob

 

[John C. Bollinger]

Eventually I realised that the only way to do it was to have a common
object the threads could report to and which could also report back to
any other thread. Bingo! That worked well - once I spelled synchronize
with a 'ze' rather than an 'se' - and then the penny dropped all the
way. There was no other way for it to work.

No other good way, at least. You've got it.

Coming back to the original problem that was really solved (for me) by
re-initialising the passed array. Did that really happen because
arrays are passed by reference?

Yes. (Well, technically, you are passing its reference by value.)
Array instances are objects like any other; when you "pass an array" you
are actually passing its reference.

Array classes are a bit special, but not so as you would notice 99% of
the time. It is sometimes useful to know that Array classes are all
Cloneable and Serializable, and that they expose their clone() methods
with public access. Because arrays are objects you can pass instances
to any method that takes a parameter of type Object; among the more
useful of these are the various methods of the Collections classes.
(I.e. you can store arrays in Lists, Sets, and Maps.)


John Bollinger
(e-mail address removed)