2

Question

The following while loop is not checking if "stopping" is true:

while (jobs.isEmpty()) {
   if (stopping) {
      running = false;
      break;
   }
}

however, if I insert any other statement before the if-statement, the if statement is checking reliably, like in the following loop:

while (jobs.isEmpty()) {
   try {
      Thread.sleep(1000);
   } catch (InterruptedException e) {
      e.printStackTrace();
   }
   if (stopping) {
      running = false;
      break;
   }
}

of course, i want it to work without the 1 sec delay, any ideas how to fix that, or why it is not working in the first place?


Clarification

To clarify it, i created a simple Test Class:

public class Test {
    static boolean stopping = false;
    static boolean running = true;

    public static void main(String[] args) throws InterruptedException {

        new Thread(() -> {
            while (true) {
                if (stopping) {
                    running = false;
                    break;
                }
            }
            System.out.println("1. Worked");
        }).start();

        new Thread(() -> {
            while (true) {
                try {
                    Thread.sleep(1000);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
                if (stopping) {
                    running = false;
                    break;
                }
            }
            System.out.println("2. Worked");
        }).start();

        Thread.sleep(1000);

        stopping = true;

        Thread.sleep(5000);
        System.exit(0);
    }
}

Output:

2. Worked

Process finished with exit code 0
4
  • 1
    Yes - you're doing nothing to ensure that the changes to stopping from one thread are visible in another. In general, tight loops like this are a bad idea - it's better to signal from one thread to another - there are lots of options here, depending on what the real use case is. The simplest fix at the moment is probably to use AtomicBoolean. (Making stopping volatile might work, but I don't remember enough details about the Java memory model to say for sure.) Commented Jan 25, 2022 at 11:17
  • @JonSkeet Making stopping volatile worked, thanks a lot! Commented Jan 25, 2022 at 11:21
  • That wasn't my recommendation. Just because it worked in your tests doesn't mean it's guaranteed to work - I wouldn't use volatile until you've read about and thoroughly absorbed everything in the (long and complicated) Java Memory Model documentation. Commented Jan 25, 2022 at 11:37
  • Rather than posting an answer in the question itself, consider posting an answer. Commented Jan 29, 2022 at 8:32

1 Answer 1

1

The immediate issue was that the update to your flag was not made visible across threads, and making the flag volatile fixed this. However, there is a better way to signal your threads to quit.

Here's an example using Thread's interrupt method instead:

public class Test {

    public static void main(String[] args) throws InterruptedException {

        Thread t = new Thread(() -> {
            while (!Thread.currentThread().isInterrupted()) {
                try {
                    System.out.println("worker is sleeping");
                    Thread.sleep(10000);
                } catch (InterruptedException e) {
                    // flag is cleared when exception is thrown,
                    //  and needs to be set again
                    Thread.currentThread().interrupt();
                }                
            }
            System.out.println("worker terminating");
        });
        t.start();
        System.out.println("main thread sleeping");
        Thread.sleep(1000);
        System.out.println("main thread interrupts worker and waits for it to finish");
        t.interrupt();
        t.join();
    }
}

The advantage of using interrupt is that things like sleep and wait know to check the flag and can react to it (such as by waking up early), which they can't do with your volatile flag.

Running your example you can see you have to wait for the worker to finish sleeping before it can check your flag and find out it needs to quit. Running this example you can see even though the worker sleeps for 10 seconds at a time it quits quickly in response to interruption.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.