Unable to stop the Java Thread

Question

I am trying to create a thread for a specific task to run in another class. The thread is starting the task but when I try to stop the thread, it is not stopping. The thread continues till the loop. Could you please help me out.

Thread Class:

package com.development;
import java.awt.Dimension;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
public class ThreadExample extends JFrame {
    MyThread mt;
    Thread th;
    ThreadExample(){        
        JPanel p1 = new JPanel();
        p1.setPreferredSize(new Dimension(400,400));
        JButton b1 = new JButton("Start");
        JButton b2 = new JButton("Stop");
        b1.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent arg0) {
                  mt = new MyThread();
                  th = new Thread(mt);
                 th.start();
            }
        });
        b2.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                th.interrupt();               
            }
        });
        p1.add(b1);
        p1.add(b2);
        this.getContentPane().add(p1);
        this.pack();
        this.setVisible(true);
    }
    public static void main(String arg[]) {
        ThreadExample mt = new ThreadExample();
        mt.setVisible(true);
    }
    public class MyThread implements Runnable{
        private volatile boolean runnable=true;
        DisplayMsg dm = new DisplayMsg("Cycle");
        @Override
        public void run() {
            while(!Thread.currentThread().isInterrupted()) {
            // TODO Auto-generated method stub
               dm.show();                 
            }    
        }
    }    
}


DisplayMsg class:

package com.development;

public class DisplayMsg {
    private String dispMsg; 
    private boolean runnable;
    DisplayMsg(String dispMsg){
        this.dispMsg=dispMsg;
    }
    public void show() {
        for(int t=0;t<100;t++) {        
        try {
            System.out.println(dispMsg + t);
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        }
    }   
}

is the downvoter still around to kindly explain the downvote? — linski, Jul 30 '13 at 14:11

score 7 · Answer 1 · answered Jul 30 '13 at 14:08

Your DisplayMsg class loops for 100 seconds, and ignores interrupts. Indeed, when Thread.sleep() is interrupted, it resets the interrupt status, then throws an InterruptedException. Since you ignore the InterruptedException, the thread continues as if nothing happened.

public void show() {
    for(int t=0;t<100;t++) {        
        try {
            System.out.println(dispMsg + t);
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
}

Don't ignore interrupts:

public void show() {
    for(int t=0;t<100;t++) {        
        try {
            System.out.println(dispMsg + t);
            Thread.sleep(1000);
        } catch (InterruptedException e) {
            // re-interrupt the thread and stop looping
            Thread.currentThread().interrupt();
            return;
        }
    }
}

It really worked. I understand the concept now, thank you very much. — user2634365, Jul 30 '13 at 14:23

Brian Roach · Answer 2 · 2013-07-30T16:56:51.557

1

When you catch the InterruptedException in your DisplayMsg class, the interrupted flag is reset.

It's fine to catch the exception, but if you need to subsequently know if the thread has been interrupted, you need to reset the flag by interrupting again with Thread.currentThread.interrupt(); in that catch block and break out of the loop / return.

edited Jul 30 '13 at 16:56

answered Jul 30 '13 at 14:08

Brian Roach

76,169
12
136
161

No, this is not necessary. `Thread.interrupt` *sets* the interrupted flag, it's a persistent change. The first thing that `Thread.sleep` does is check the flag and only then put the thread into wait state. – Marko Topolnik Jul 30 '13 at 15:09
@MarkoTopolnik Sorry, bad editing / adding early in the morning. I ended up adding the breaking/returning in the catch after adding the last part about using a conditional in the loop. Editing. – Brian Roach Jul 30 '13 at 16:56

Marko Topolnik · Answer 3 · 2013-07-30T15:32:54.433

1

A much simpler design which achieves what you need is the following:

public class MyThread implements Runnable{
    DisplayMsg dm = new DisplayMsg("Cycle");
    @Override public void run() {
        try { while(true) dm.show(); } 
        catch (InterruptedException e) { }
    }
}    

public class DisplayMsg {
  ...
  public void show() throws InterruptedException {
    for(int t=0;t<100;t++) {        
      System.out.println(dispMsg + t);
      Thread.sleep(1000);
    }
}

}

This will simply let the InterruptedException propagate and end MyThread, with no effort on your own.

edited Jul 30 '13 at 15:32

answered Jul 30 '13 at 15:16

Marko Topolnik

195,646
29
319
436

You still have to catch the InterruptedException in run(), and return in the catch block. As is, it doesn't compile. – JB Nizet Jul 30 '13 at 15:27
True! Will fix right away. Those damned checked exceptions, nothing good ever comes out of them... – Marko Topolnik Jul 30 '13 at 15:31
@JBNizet Note that I don't really have to return from the catch block. It returns with or without it. – Marko Topolnik Jul 30 '13 at 15:34
It all depends if the try/catch is inside or outside the while loop. Since I didn't know where you intended to put it... The important thing is to return from the run method, implicitely or explicitely. But we both know that we both know that :-) – JB Nizet Jul 30 '13 at 15:37
@JBNizet It was just a remark on how to save yet another keyword, not a correction of your statement :) – Marko Topolnik Jul 30 '13 at 15:40

Chris Hinshaw · Answer 4 · 2013-07-30T14:16:52.013

I believe what is happening is that you need to be checking !Thread.currentThread().isInterrupted() before each sleep. You are calling show() in the method which will iterate for 100 seconds only then will the your while loop check to see if the thread is interrupted. Move your check for interrtuped to your show method and see if it sees the interrupted flag. As per the next answers you should also set the Thread interrupted flag when you catch the InterruptedException.

Unable to stop the Java Thread

4 Answers4