Morteza Mashayekhi
Morteza Mashayekhi

Reputation: 934

Java Thread suspend does not work

This is a sample code using java thread. The problem is that I cannot suspend the thread.

public class RunnableDemo implements Runnable {
   public Thread t;
   private String threadName;
   boolean suspended = false;
   RunnableDemo( String name){
       threadName = name;
       System.out.println("Creating " +  threadName );
   }
   public void run() {
      System.out.println("Running " +  threadName );
      try {
         while (true) {
            System.out.println("Thread: " + threadName + ", " + i);
            // Let the thread sleep for a while.
            Thread.sleep(300);
            synchronized(this) {
               while(suspended) {
                  wait();
               }
            }
        }
     } catch (InterruptedException e) {
         System.out.println("Thread " +  threadName + " interrupted.");
     }
     System.out.println("Thread " +  threadName + " exiting.");
   }
   public void start ()
   {
      System.out.println("Starting " +  threadName );
      if (t == null)
      {
         t = new Thread (this, threadName);
         t.start ();
      }
   }
   public void suspend() {
      suspended = true;
   }

The suspend function below does not work:

RunnableDemo R1 = new RunnableDemo( "Thread-1");
R1.start();
R1.suspend();  // This line does not work and thread continues ...

In fact the thread does not see the new value for suspended when I call suspend() . Any idea ?

Upvotes: 2

Views: 1239

Answers (3)

Gee Bee
Gee Bee

Reputation: 1794

Using wait() might not work as you expect. The wait/notify is built as a simple implementation of the sempaphore (as an object instance wide monitor) context of concurrent programming. Calling notify() from any other thread not locking the monitor of the thread object instance won't work, see Java Wait and Notify: IllegalMonitorStateException

You can easily achieve your goal by checking the suspended flag regularly, and just wait a bit if your thread shall not do anything:

public class SuspendDemo implements Runnable {
    public Thread t;
    private final String threadName;
    boolean suspended = false;
    SuspendDemo(final String name){
        threadName = name;
        System.out.println("Creating " +  threadName );
    }

    @Override
    public void run() {
        System.out.println("Running " +  threadName );
        try {
            while (true) {
                System.out.println("Thread: " + threadName);
                // Let the thread sleep for a while.
                Thread.sleep(300);
                while(suspended) {
                    System.out.println("suspended");
                    Thread.sleep(50);
                }
            }
        } catch (final InterruptedException e) {
            System.out.println("Thread " +  threadName + " interrupted.");
        }
        System.out.println("Thread " +  threadName + " exiting.");
    }

    public void start () {
        System.out.println("Starting " +  threadName );
        if (t == null) {
            t = new Thread (this, threadName);
            t.start ();
        }
    }

    void suspend() {
        suspended = true;
    }

    void resume() {
        suspended = false;
        notify();
    }

    public static void main(final String[] args) throws InterruptedException  {
        final SuspendDemo R1 = new SuspendDemo( "Thread-1");
        R1.start();
        R1.suspend();
        Thread.sleep(500);
        R1.resume();
    }
}

Resulting:

> Creating Thread-1 
> Starting Thread-1 
> Running Thread-1 
> Thread: Thread-1
> suspended
> suspended
> suspended
> suspended
> Thread: Thread-1
> Thread: Thread-1
> Thread: Thread-1

The suspend flag is a native boolean, so writing it is an atomic operation, therefore you can go without synchronization on anything. I suggest to fully understand concurrent programming and the monitor concurrency model what Java uses, before using synchronized. I found using synchronize(this) blocks hard to maintain and prone to mistakes.

The next example solves the problem with wait() and notifyAll(). Note the use of synchronized methods for locking the monitor of the thread object instead of the synchronize(this) blocks.

public class SuspendDemo implements Runnable {
    public Thread t;
    private final String threadName;
    boolean suspended = false;
    SuspendDemo(final String name){
        threadName = name;
        System.out.println("Creating " +  threadName );
    }

    @Override
    public void run() {
        System.out.println("Running " +  threadName );
        try {
            while (true) {
                System.out.println("Thread: " + threadName);
                // Let the thread sleep for a while.
                Thread.sleep(300);
                work();
            }
        } catch (final InterruptedException e) {
            System.out.println("Thread " +  threadName + " interrupted.");
        }
        System.out.println("Thread " +  threadName + " exiting.");
    }

    synchronized protected void work() throws InterruptedException {
        while(suspended) {
            System.out.println("suspended");
            wait();
            System.out.println("resumed");
        }
    }

    public void start () {
        System.out.println("Starting " +  threadName );
        if (t == null) {
            t = new Thread (this, threadName);
            t.start ();
        }
    }

    synchronized void suspend() {
        suspended = true;
        notifyAll();
    }

    synchronized void resume() {
        suspended = false;
        notifyAll();
    }

    public static void main(final String[] args) throws InterruptedException  {
        final SuspendDemo R1 = new SuspendDemo( "Thread-1");
        R1.start();
        R1.suspend();
        Thread.sleep(500);
        R1.resume();
    }
}

Upvotes: 1

Simon Fischer
Simon Fischer

Reputation: 1196

It is not absolutely clear to me what you are trying to achieve, but here are a few observations and tips that may help:

  • By the time the main thread has completed the suspend() method, the second thread, "Thread-1", has probably not even entered the loop. You will see this if you add more debug output in suspend() and before and after wait(). The opposite could also be true, and you would reach the call to wait() before suspend is set to true. Then, once wait() is called, you won't see any more updates in "Thread-1" since it is waiting.
  • The wait() method will never return. To achieve this, synchronize on the same object that you synchronize on when calling wait(), i.e. R1 in the main method and call notifyAll() on this object. You can also do that inside your suspend() method.
  • This approach makes methods like Thread.sleep() pretty much unnecessary. Calling wait() is no active waiting and won't consume significant resources.

In short, change your suspend-method to

void suspend() {
   suspended = true;
   synchronized (this) {
       this.notifyAll();
   }
}

This will change how your program behaves and make it more clear what happens. But either way, it does not seem like "suspend" is a good name for what the method does, before or after, but if you experiment with this you may get a better understanding of how you can achieve what you want to achieve.

Upvotes: 1

Jean Logeart
Jean Logeart

Reputation: 53809

Declare suspended as volatile:

volatile boolean suspended = false;

Upvotes: 3

Related Questions