CODEWITHSUNDEEP
javathreadpool
teodozjan
teodozjan

Reputation: 913

Impossible to cancel running task with SingleThreadExecutor

I have class implementing Runnable:

public class Abc implements Runnable{
    public synchronized void run(){
        while(!(Thread.currentThread().isInterrupted() || someCondition())){
           doSomething();
           try{
               this.wait(SOME_TIME);
           } catch(InterruptedException ex){
            logger.error("Ex",ex);
            Thread.currentThread.interrupt();
               }
}}}

After submiting it to threadpool I would like to cancel it, with interrupt.

futureTask.cancel(true);

But today my Runnable object stopped receiving interrupt. I've logged out interrupt state : false. InterruptedException is not thrown. Any suggestions what is wrong?

Upvotes: 0

Views: 718

Answers (3)

teodozjan
teodozjan

Reputation: 913

Solved

This was actually very tricky race condition. I've been starting task from gui thread and cancelling it from network reader thread. I cannot explain why log4j didn't display loggers but better thread control resolved my issue. Your answers helped me to track an idea to solve it, thanks.

When something worked yesterday but not today - probably its race condition or deadlock. :-)

Upvotes: 0

Tomasz Stanczak
Tomasz Stanczak

Reputation: 13164

First you snippet doesn't even compile: you are lacking braces, Thread.currentThread is a method not an instance variable. Then, are you calling Thread.interrupted() in your doSomething method? If yes: it clears the interrupted state of a thread, so that wait won't be interrupted and currentThread().isInterrupted() will return false.

You also don't need to recheck interrupted state in the while clause or reinterrupt the thread once it got iterrupted. This looks nicer (and catches exception that your doSomething might throw):

  public synchronized void run() {
    try {
      while (!someCondition()) {
        doSomething();
        wait(SOME_TIME);
      }
    } catch (InterruptedException ie) {
      logger.error(ie);
    } catch (Exception e) {
      logger.error(e);
    }
  }

@Peter: moving try/catch around while is definitely a good suggestion!

Upvotes: 0

Peter Lawrey
Peter Lawrey

Reputation: 533492

My guess is that the interrupt was consumed by doSomething or doSomething was tied up (not returning)

Using wait() in a loop like this suggest to me that you have a control flow which would be better suited to using the Concurrency libraries. Can you give some more details as to what you are waiting for?

BTW, If you move the try/cath to outside the loop it would simplify it.

Upvotes: 1

Related Questions