0

Can someone please explain to me what I am missing: when I call Thread.sleep(1000) I suppose both threads should be executed in 1s so after that why should I make doSlice false to stop threads in 1s why Thread.sleep() just doesn't stop them in 1s. I mean after 1s run method even shouldn't be called to check while condition:

public class ExecutionScheduling  extends Thread{
    public int slice_count=0;
    public boolean doSlice=true;
    public String name;

    public ExecutionScheduling(String name){
        this.name=name;
    }

    public void run() {
        while (doSlice) {
            slice_count++;
            System.out.println(name);
        }
    }

    public static void main(String[] args) throws InterruptedException {
        ExecutionScheduling executionScheduling=new ExecutionScheduling("ex");
        ExecutionScheduling executionScheduling1=new ExecutionScheduling("ex1");

        executionScheduling.start();
        executionScheduling1.start();

        Thread.sleep(1000);
        executionScheduling.doSlice=false;
        executionScheduling1.doSlice=false;

        System.out.println("ex: "+executionScheduling.slice_count);
        System.out.println("ex1: "+executionScheduling1.slice_count);
    }
}
Gray
  • 115,027
  • 24
  • 293
  • 354
saba
  • 332
  • 2
  • 14
  • 2
    `Thread.sleep(1000)` does not _do_ anything at all: It does nothing at all for one full second, and then it returns. – Solomon Slow Jul 23 '20 at 12:41
  • In addition, the code is not thread-safe. – Kayaman Jul 23 '20 at 12:42
  • I get that so the main thread does nothing for 1s and two chid threads continue running after that when doSlice changes to false child threads will be stoped. – saba Jul 23 '20 at 12:50
  • 1
    In order to guarantee that a doSlice change is observed by other threads you should make it volatile. Otherwise JVM can easily treat that loop as endless. – suff Jul 23 '20 at 13:15

1 Answers1

3

Can someone please explain to me what I am missing

What you are missing is memory synchronization between the threads. When you start your 2 background threads, they have their own local memory (on their own CPUs) and you need to specifically update any shared data between them.

What is complicating the code is that System.out.println(...) is a synchronized method so it is giving you some memory synchronization "for free" but you shouldn't depend on it (see below). This means that if you removed that debug code, your program is going to be behave differently. Be careful of any usage of System.out.print* in threaded code.

Thread.sleep(1000);

When you run this sleep command, it will cause the main thread to go to sleep but the 2 background threads continue to run. They are each updating their own copy of slice_count but there is no guarantee that the main thread will see these updates.

// need to add the volatile keyword here
private volatile int slice_count;

By adding the volatile Java keyword to slice_count, this marks the field as being accessed by multiple threads. When the main thread the accesses slice_count, it will read the most updated value of it. You might also want to look into AtomicInteger which wraps a volatile int but allows multiple threads to do stuff like incrementAndGet().

Another place where you have memory sync issues is:

executionScheduling.doSlice = false;
executionScheduling1.doSlice = false;

So the doSlice field also needs to be volatile:

// need to add the volatile keyword here
public volatile boolean doSlice = true;

And lastly, in no order:

  • Your fields should be private if at all possible.

  • It should be executionScheduling1 and executionScheduling2.

  • It is a better pattern to define a Runnable instead of a Thread. See: https://stackoverflow.com/a/541527/179850

  • You might consider doing a join() to wait or each of the threads to finish their work before you print out their results.

    // set the doSlices to false
    executionScheduling.join()
    executionScheduling1.join()
    // printf results
    

    If you add the join() calls then this will handle the memory synchronization for you in terms of slice_count so those no longer need to be volatile as long as you access them after the join() calls finish. Yes this is confusing. Thread coding is non-trivial. You will still need the doSlice fields to be volatile because they are accessed before join() finishes.

Gray
  • 115,027
  • 24
  • 293
  • 354
  • 1
    This. Great answer. Plus, @saba: grabbing a good book on Java concurrency (which is far from trivial) is a good start. Reading e.g. Java Concurrency in Practice by Brian Goetz is heplful, and getting knowledge on Java's memory model is also a good thing to do. A good point to start is Rafael Winterhalter's presentation - The Java Memory Model for Practitioners available on YouTube. – ttarczynski Jul 23 '20 at 22:08