Reputation: 98
When a prime number is found, I have to stop it. When I use synchronized before the while
, only one thread process will occur. However, multiple thread operations should occur, but all should stop when prime is found.
The initial value of i
in the control part has been changed.
What I want to do is to find prime numbers using lock and synchronized.
public abstract class NumberGenerator {
private boolean isStop;
public abstract int generateNumber();
public void stop() {
this.isStop = true;
}
public boolean isStopped() {
return isStop;
}
}
public class IntegerNumberGenerator extends NumberGenerator {
private Random random;
int randomAtama;
public IntegerNumberGenerator() {
this.random = new Random();
}
@Override
public int generateNumber() {
return random.nextInt(100) + 1;
}
}
public class PrimeNumberChecker implements Runnable {
private NumberGenerator generator;
private Lock lock = new ReentrantLock();
public Condition continueLock = lock.newCondition();
public PrimeNumberChecker(NumberGenerator generator) {
this.generator = generator;
}
@Override
public void run() {
while (!generator.isStopped()) {
int number = generator.generateNumber();
System.out.println(Thread.currentThread().getName() + " generated " + number);
if (check(number)) {
System.out.println(number + " is prime !");
generator.stop();
}
}
}
public static boolean check(int number) {
boolean result = true;
for (int i = 2; i <= number / 2; i++) {
if ((number % i) == 0) {
result = false;
}
}
return result;
}
}
public class Driver {
public static void main(String[] args) {
ExecutorService executorService = Executors.newCachedThreadPool();
NumberGenerator numberGenerator = new IntegerNumberGenerator();
for (int i = 0; i < 5; i++) {
executorService.execute(new PrimeNumberChecker(numberGenerator));
}
executorService.shutdown();
}
}
Upvotes: 4
Views: 168
Reputation: 51473
You can optimized the check method to:
public static boolean check(int number) {
for (int i = 2; i <= number / 2; i++) {
if ((number % i) == 0) {
return false;
}
}
return true;
}
as soon as you know that the number is not a prime you can just return earlier.
When I find a prime number, I have to stop it. If I use it before synchronized while, only one thread process will occur. Multiple thread operations should occur, but should stop when prime is found.
You can achieve this by adding first volatile to the isStop
flag:
private volatile boolean isStop = false;
then checking for generator.isStopped()
(also) in the method that finds out if a number is prime:
public boolean check(int number) {
for (int i = 2; i <= number / 2; i++) {
if (generator.isStopped() || number % i == 0) {
return false;
}
}
return true;
}
Finally, you need to synchronize while reading the value of check
method because it might happen that multiple threads find the prime number at the same time. So adapt your code to:
boolean result = check(number); // All threads to work in parallel
synchronized (generator) {
if (result && !generator.isStopped()) {
System.out.println(number + " is prime !");
generator.stop();
}
}
Volatile here is not enough because multiple threads might managed to enter inside the block of code within the
if(result && !generator.isStopped())
before one of them is able to actually call generator.stop()
;. Making the variable isStop
AtomicBoolean
alone would also not help, because of the exact same reason.
The point is the statements !generator.isStopped()
and generator.stop();
have to be performed within the same critical region, either using synchronized or performing the two operations atomically in the same go. Therefore, for the AtomicBoolean
to work you would have to do the following:
public abstract class NumberGenerator {
private final AtomicBoolean isStop = new AtomicBoolean(false);
public abstract int generateNumber();
public void stop() {
this.isStop.set(true);
}
public boolean isStopped() {
return isStop.get();
}
public boolean getAndSet(){
return isStop.getAndSet(true);
}
}
and
if (check(number) && !generator.getAndSet()) {
System.out.println(number + " is prime !");
}
Because the getAndSet
is done atomically, you do not run the risk of having multiple threads printing out their primes.
What I want to do is to find prime numbers using lock and synchronized.
If you mean using only one or the other (since you do not need to use both) then you can do the following:
boolean result = check(number);
synchronized (generator) {
if (result && !generator.isStopped()) {
System.out.println(number + " is prime !");
generator.stop();
}
}
this would work even without the volatile.
Upvotes: 1
Reputation: 642
Instead of having the logic of "when it should stop" in the abstract class, the Thread itself should be aware of it. So I would add an AtomicBoolean as a flag inside of the Runnable class and a stop() method to deal with it. Then the run method would be looking for the AtomicBoolean to stop whenever this boolean changes.
Something like this:
public class PrimeNumberChecker implements Runnable {
//Flag to control the running
private final AtomicBoolean isRunning = new AtomicBoolean(false);
private NumberGenerator generator;
private Lock lock = new ReentrantLock();
public Condition continueLock = lock.newCondition();
public PrimeNumberChecker(NumberGenerator generator) {
this.generator = generator;
}
@Override
public void run() {
isRunning.set(true);
while (isRunning.get()) {
int number = generator.generateNumber();
System.out.println(Thread.currentThread().getName() + " generated " + number);
if (check(number)) {
System.out.println(number + " is prime !");
generator.stop();
}
}
}
// So you can stop it from the outside
public void stop() {
isRunning.set(false);
}
public static boolean check(int number) {
boolean result = true;
for (int i = 0; i <= number / 2; i++) {
if ((number % 2) == 0) {
result = false;
}
}
return result;
}
}
Upvotes: 0