Stopping a Thread inside a web container

Question

I have a thread that is started when my web application starts (contextInitialized). All this thread does is, read data from the socket.

public void run() {
    while (!Thread.currentThread().isInterrupted() || !this.isInterrupted()
            || !stopped) {
        try {
            System.out.println("************polling..............");
            readData();
            Thread.sleep(300);
        } catch (InterruptedException e) {
            System.out.println("interrupted... ");
            Thread.currentThread().interrupt();
            break; //required?
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

   public void shutdown() {
    stopped = true;
    }
    // I know I'm blocking here.
    private void readData() {
          if (null == socket)
             throw new ConnectionException();
          if (inStream == null) {
             inStream = this.socket.getInputStream();
          }
          byte[] buff = new byte[length];
          int receiveLength = 0;
          do {
             try {
              receiveLength = inStream.read(buff, 0, length);
              } catch (Exception e) {
               e.printStackTrace();
              }

             } while (receiveLength <= 0);
       }

I call interrupt() method in the contextDestroyed, but this thread fails to stop, why is that?

    if (poller != null) {
        ((Poller) poller).shutdown();
        poller.interrupt();
                    // should I need to do this?
        while (poller.isAlive()) {
            ;
        }
    }

You do not actually need the `shutdown` method at all. `interrupt` is the already-existing interface method for telling a thread to stop, so get rid of `shutdown`. — AJMansfield, Mar 20 '14 at 15:44
That really doesn't matter. You should do it the right way, and the figure out what you did that makes the right way not work. Also, how exactly are you invoking the thread? I'd do it by submitting it to an `ExecutorService`, like `Executors.newSingleThreadExecutor`, but some people seem to like making their own `Thread` objects, even though that is not usually the best way. — AJMansfield, Mar 20 '14 at 15:46
How are you interrupting this thread? Are we sure that it's the reader thread that's being interrupted? — Isaiah van der Elst, Mar 20 '14 at 15:50
Yes, I have the instance of this thread in my ServletContextListener implementation class. — vvra, Mar 20 '14 at 15:53
Are you seeing "interrupted... "? If I had to guess "this.isInterrupted()" always returns false. — Isaiah van der Elst, Mar 20 '14 at 16:01
Also, "Thread.currentThread().interrupt();" should have no effect. You can remove that from the run method. — Isaiah van der Elst, Mar 20 '14 at 16:04

AJMansfield · Accepted Answer · 2014-03-20T18:57:53.393

If I were doing this, this is what I would do it using different library stuff:

The real problem is this loop inside readData:

do {
    try {
        receiveLength = inStream.read(buff, 0, length);
    } catch (Exception e) {
        e.printStackTrace();
    }
} while (receiveLength <= 0);

If you use a loop here, it will keep reading even if there is nothing to read, rather than returning to the outer loop with the delay. Get rid of the loop. Also, move all the initialization in the method to the constructor.

Here is how I would do this:

class ConnectionReader implements TimerTask {

    InputStream inStream;
    byte[] buf;

    public ConnectionReader(Socket socket, int length){
        if (null == socket)
            throw new ConnectionException();
        if (inStream == null) {
            inStream = this.socket.getInputStream();
        }
        buf = new byte[length]
    }

    @Override
    public void run() {
        try {
            System.out.println("polling...");
            readData();
        } catch (InterruptedException e) {
            System.out.println("interrupted...");
            cancel()
        } catch (Exception e) {
            e.printStackTrace();
        }
    }


    private void readData() {
       try {
            receiveLength = inStream.read(buff, 0, length);
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

To start it, use a Timer set for the desired 300ms delay:

TimerTask readTask = new ConnectionReader(this.socket, length);
new Timer().schedule(readTask,0,300);

To cancel it, you just call readTask.cancel().

I like this approach, I will try rewriting my thread like this. But as you have mentioned earlier it about how I create thread, so I could now stop the thread, I was blocking the thread reading nothing from the socket. — vvra, Mar 20 '14 at 16:37

Stopping a Thread inside a web container

1 Answers1