0

I need to manage many background threads that do something with some objects as key but only one thread can work with same object at the same time, For example while thread A is working on object A, if thread B called for working with object A, thread A should be canceled before thread B can run.

Here is my code, but when I run, The first thread doesn't stop and continues with second one:

private static ExecutorService mExecutor = Executors.newCachedThreadPool();
private static ConcurrentMap<Object, Future> mRunningTasks = new ConcurrentHashMap<>();
public static void run(final Runnable runnable, final Object key) {
    Future<?> future = mRunningTasks.get(key);
    if (future != null) {
        future.cancel(true);
    }
    future = new FutureTask<>(new Runnable() {
        @Override
        public void run() {
            //How to handle InterruptedException here ?
            //while there's no potential to throw an InterruptedException
            runnable.run();
            mRunningTasks.remove(key);
        }
    }, null);
    mRunningTasks.put(key, future);
    mExecutor.execute((FutureTask) future);
}

What am I missing here ?

Thanks in advance.

PS: I need a behavior like Picasso API when cancels requests using an ImageView as a key object.

Farshad
  • 3,074
  • 2
  • 30
  • 44

4 Answers4

0

The docs for Future.cancel() say that it attempts to cancel the task, but may be unsuccessful. You need to check the boolean return value to see if it actually cancelled.

GreyBeardedGeek
  • 29,460
  • 2
  • 47
  • 67
  • Hey, Thanks for response, If I mistake not the returning value just shows the operation result, although when I log, the result is true but the thread keeps running, What I am asking is how and where I can stop the runnable object? – Farshad Jun 16 '16 at 21:28
  • You can't kill a running Thread in Java (well you can but it's a very bad practice). The right way is to set a volatile boolean variable to your instance to ask it to terminate its processing. The ThreadPools are meant for short lived running Threads. – Guillaume F. Jun 16 '16 at 21:42
  • @GuillaumeF. I get what you're suggesting, Check the condition in a loop but on the first repeat the whole code in runnable will execute If you understand what I mean – Farshad Jun 16 '16 at 21:53
  • Well, you can cancel a FutureTask if it hasn't started. If you can't cancel it, it means it's too late. It is already running. – Guillaume F. Jun 16 '16 at 21:55
  • Sure, What I am struggling with is this fact that in this way the run method will be canceled before running. It is considered as a line of code! although it contains many lines of code within, I need to cancel the method even in the middle of it. – Farshad Jun 16 '16 at 22:03
  • The approach you mentioned is more proper to the situations that we can break the task into repeats of loops but in my case It's not the best way. right ? – Farshad Jun 16 '16 at 22:07
0

There are multiple other raceconditions in your code.

ConcurrentHashMap is threadsafe, but the way you are using it is not. You might: * cancel a future multiple times * add a future without canceling the previous one * removing the wrong futures from the map

You should be more specific about you actually want to solve. There might be better solutions to that problem.

k5_
  • 5,450
  • 2
  • 19
  • 27
0

You want to kill the Thread (Thread.stop()), which is a very bad practice and deprecated for many reasons... Not only for don't have responsibility over the Thread, since it is managed by the ThreadPool, but killing it ruins the concept of atomicity of your algorithm, leaving your software in a possibly undetermined state.

You can cancel a FutureTask if it hasn't started. If you can't cancel it, it means it is already running. This occurs incredibly fast on modern multi-cores processors.

The ThreadPool is usually meant for short lived running Threads. But in your case, the right way to end the task properly is to set a volatile boolean variable in your Task Class and check it regularly to leave the method.

InterruptedException is only thrown when your Thread is waiting or sleeping, which is not present in your current demonstration code. If your Thread actually is waiting or sleeping, then send a .notify() to the monitor Object to interrupt it, and catch the Exception in a try {} catch()

Guillaume F.
  • 5,905
  • 2
  • 31
  • 59
0

There are two primary problems here 1) the run method needs to be declared synchronised (avoids race conditions when checking the state of things) and 2) future.cancel is not capable of halting work that has already started. So add your own guards/cancelling mechanism to the runnables.

That said, the cleanest solution is to avoid the need to cancel in the first place, however there is not enough domain detail in this question to verify if that would be feasible. A common pattern for this kind of problem is to return a promise from run and cache the promises. A promise is a thread safe container object for the result of an asynchronous operation. If the same work is scheduled twice then the same promise may be returned. If a promise has a value it would be possible to see how old the value was to determine the scheduling behaviour, such as return the old value, return the old value and schedule an update of the value behind the scenes or throw the old value away as too old and fetch a clean value.

Chris K
  • 11,622
  • 1
  • 36
  • 49