19

I have a Service that launches a Thread and a Runnable like so.

t = new Thread(new Runnable() {
    public void run() {
        doSomething();
    }
});

t.start();

The reason for the thread is to perform an Async task doSomething(). For now lets not worry about the other class AsyncTask. I have tried it and it does not work for my case. Edit: I can't use AsyncTask because it is meant for the UI thread only. This piece of code has to operate inside a Service, so nope, no AsyncTask :(

doSomething() contains some external libs so the issue I am having is that it can potentially be hung at one of the commands, without return any value (hence no error checking can even be done)

To work around this, I will want to, at some point of time, destroy the Service.

stopService(new Intent("net.MyService.intent));

This works fine and is easily verified on the phone. However, the Thread which was created above will continue to run even when the Service that spawned it is destroyed.

I am thus looking for the correct commands to insert in the Service's onDestroy() which will clean up the Thread for me.

t.destroy();
t.stop();

are both depreciated and cause application crashes.

I took this code from somewhere

@Override
public void onDestroy() {

    Thread th = t;
    t = null;

    th.interrupt();

    super.onDestroy();
}

but it still does not work, the thread continues to run. Any help guys?

foodman
  • 191
  • 1
  • 1
  • 4

7 Answers7

11

The thread destroy and stop methods are inherently deadlock prone and not safe. Their existence also gives the illusion that there might be some way of halting another thread immediately when something else tells it to.

I understand your thinking, from your point of view their is one main thread, and when this thread hasn't received a response from it's worker thread in a while you'd like to kill it and restart it, without caring what it's up to. But the reason those methods are deprecated is you should care what the thread is up to. A lot.

What if the thread has a lock around a variable you need to use later? What if a thread has a file handle open? In all these cases, and many more, simply stopping the thread at it's current operation would leave things in mess -- quite likely your application would just crash further down the line.

So in order for a thread to be interruptible or cancel-able or stoppable, it has to manage this itself. If a thread or operation provides no way for itself to be interrupted, then you cannot interrupt it - it is assumed to do so would be unsafe.

If you runnable is literally

public void run() {
   doSomething();
}

then there is no way to interrupt it. One would hope that if doSomething were a long operation that there might be a way to either interact with it incrementally with something like

public void run() {
   while (running) {
       MyParser.parseNext();
   }
}

or to be able to pass in a variable by reference which indicates whether the thread is interrupted or not, and hopefully the method would interrupt itself at suitable location.

Remember a blocking operation is blocking. There is no way to get around that, you cannot cancel it part way through.

Joseph Earl
  • 23,351
  • 11
  • 76
  • 89
  • Thanks for the reply. The current ONLY reason why I'm using a Thread is because the doSomething() operations are asynchronous and will hang the main thread on the phone, making the whole phone unusable during that time. It actually does not involve any variables that I need, and does not interact with anything outside the thread. I'll look further into the AsyncTask you mentioned before – foodman Apr 15 '11 at 01:47
  • @Joseph Earl Have you solved your issue? How did you stop Thread execution in your Service? I have the same issue here, if you can help: https://stackoverflow.com/review/suggested-edits/20715706 – yozhik Aug 29 '18 at 10:51
2

Alternative answer

Use the following code:

MyThread thread;     // class field

Create and start the thread as you do it right now.

thread = new MyThread();
thread.start();

When the service is destroyed, "signal" the thread to quit

public void onDestroy() {
    // Stop the thread
    thread.abort = true;
    thread.interrupt();
}

Here is thread implementation

//another class or maybe an inner class
class MyThread extends Thread {
    syncronized boolean abort = false;

    //ugly, I know
    public void run() {
       try {
           if(!abort) doA();
           if(!abort) doB();
           if(!abort) doC();
           if(!abort) doD();
       } catch (InterruptedException ex) {
          Log.w("tag", "Interrupted!");
       }
    }
}

You might want to read the following:

I think that you could rely on catching the exception and not check abort but I decided to keep it that way.

UPDATE

I've seen this sample in codeguru:

public class Worker implements Runnable {
    private String result;
    public run() {
        result = blockingMethodCall();
    }
    public String getResult() {
        return result;
    }
}

public class MainProgram {
    public void mainMethod() {
        ...
        Worker worker = new Worker(); 
        Thread thread = new Thread(worker); 
        thread.start();
        // Returns when finished executing, or after maximum TIME_OUT time
        thread.join(TIME_OUT); 
        if (thread.isAlive()) {
            // If the thread is still alive, it's still blocking on the methodcall, try stopping it
            thread.interrupt();
            return null;
        } else {
            // The thread is finished, get the result
            return worker.getResult(); 
        }
    }
}
Community
  • 1
  • 1
Pedro Loureiro
  • 11,436
  • 2
  • 31
  • 37
  • The issue he is having the that the equivalent of the `doA()` function is not returning cleanly and thus the `Thread` is basically hung on that method call. As such it isn't getting to the next step, which would be the `if(!abort)` call. – dave.c Apr 14 '11 at 11:03
  • but if he calls thread.interrupt() it should raise an exception that will be caught by the try-catch clause. – Pedro Loureiro Apr 14 '11 at 11:13
  • Yeah, I'd have thought so too, but he is saying he called `th.interrupt();` in `onDestroy()` and the thread continues to run. – dave.c Apr 14 '11 at 11:49
  • 1
    thread.interrupt() only raises an exception if the thread is processing Thread.wait(), Thread.sleep(), or Thread.join(). If you were to put other commands into your **try** loop, you won't be able to **catch InterruptedException**. This doesn't mean that I should then put one of these (e.g. sleep()) into my Thread, because while my Thread is hung at doA(), calling interrupt() does not raise the InterruptedException. It only sets the flag isInterrupted() which is useless because the Thread is unable to recover from doA() and 'check' isInterrupted() – foodman Apr 15 '11 at 01:52
  • oh, I didn't know that. Anyway, a little more googling and a new update in my answer. Are we lucky this time? – Pedro Loureiro Apr 18 '11 at 10:41
  • Don't synchronize on boolean variables! –  Sep 28 '14 at 13:13
  • Hey, I have a thread with a simple listener in it. If I unregister the listener, can I just call `thread.inturrupt()` to make sure it gets destroyed? – Ruchir Baronia Feb 07 '16 at 03:29
1

Did you check the Java Thread Primitive Deprecation Documentation which is referenced in the Thread API JavaDoc. You will find some hints to handle your problem.

FrVaBe
  • 47,963
  • 16
  • 124
  • 157
  • 3
    That link basically re-iterates all the problems and constraints I am faced with. The link talks about how to use interrupt() in place of stop(), which is what I'm doing. The problem with interrupt() is that it only works on sleep(), wait() and join(). In my code, the line that hangs is none of these, and it never recovers, not even after an hour. the thread is stuck at that line, and no interrupt can rescue it out. – foodman Apr 14 '11 at 09:05
1

why don't you use an AsyncTask?

A task can be cancelled at any time by invoking cancel(boolean). Invoking this method will cause subsequent calls to isCancelled() to return true. After invoking this method, onCancelled(Object), instead of onPostExecute(Object) will be invoked after doInBackground(Object[]) returns. To ensure that a task is cancelled as quickly as possible, you should always check the return value of isCancelled() periodically from doInBackground(Object[]), if possible (inside a loop for instance.)

Pedro Loureiro
  • 11,436
  • 2
  • 31
  • 37
  • 2
    I can't use AsyncTask because of this : "It is also very important to remember that an AsyncTask instance has to be created on the UI thread and can be executed only once" (developer.android.com/resources/articles/…). My thread/async task runs on a Service so it can't happen – – foodman Apr 14 '11 at 09:12
  • 1
    @foodman: I think you misunderstood what they were trying to say -- that is only **if** you perform operations which affect the UI thread from the `onPostExecute` etc methods. If you don't touch the UI thread (which you won't be since you're running it in a Service) then it's fine to use an AsyncTask. – Joseph Earl Apr 14 '11 at 09:28
0

I like to take the following approach:

class MyHandler extends Handler {
    final Semaphore stopEvent = new Semaphore(0);

    @Override
    public void handleMessage(Message msg) {
        try {
            while (!stopEvent.tryAcquire(0, TimeUnit.SECONDS)) {
                doSomething();

                if (stopEvent.tryAcquire(SLEEP_TIME, TimeUnit.MILLISECONDS)) {
                    break;
                }
            }
        } catch (InterruptedException ignored) {
        }

        stopSelf();
    }
}

On service onDestroy just release the stopEvent:

@Override
public void onDestroy() {
    myHandler.stopEvent.release();
    myHandler = null;

    super.onDestroy();
}
Skarllot
  • 745
  • 6
  • 16
0

Better to use global variable stopThread, stop thread once variable changed to true.

btnStop.setOnClickListener(new OnClickListener() {          
        @Override
        public void onClick(View arg0){
            stopThread = true;
        }
    }); 


public void run() {
        while (!stopThread) {
           //do something
        }
}
Shiv Buyya
  • 3,770
  • 2
  • 30
  • 25
0

I think the best way to create and communicate with another thread is to use an AsyncTask. Heres an example of one:

public class Task extends AsyncTask<Void, Void, Void> {

    private static final String TAG = "Task";

    private boolean mPaused;

    private Runnable mRunnable;

    public Task(Runnable runnable) {
        mRunnable = runnable;
        play();
    }

    @Override
    protected Void doInBackground(Void... params) {
        while (!isCancelled()) {
            if (!mPaused) {

                mRunnable.run();
                sleep();
            }
        }
        return null;
    }

    private void sleep() {
        try {
            Thread.sleep(10);
        } catch (InterruptedException e) {
            Log.w(TAG, e.getMessage());
        }
    }

    public void play() {
        mPaused = false;
    }

    public void pause() {
        mPaused = true;
    }

    public void stop() {
        pause();
        cancel(true);
    }

    public boolean isPaused() {
        return mPaused;
    }

}

You can now easily use this class, and start the thread by writing:

Task task = new Task(myRunnable);
task.execute((Void) null);

Along with this you can easily pause or stop the thread from looping:

Example of pausing and playing the thread:

mButton.setOnClickListener(new View.OnClickListener() {
    @Override
    public void onClick(View v) {
        if (task.isPaused()) {
            task.play();
        } else {
            task.pause();
        }
    }
});

Example of stopping and starting the thread:

mButton.setOnClickListener(new View.OnClickListener() {
    @Override
    public void onClick(View v) {
        if (task.isCancelled()) {
            task = new Task(myRunnable);
            task.execute((Void) null);
        } else {
            task.stop();
        }
    }
});
Anthony Cannon
  • 1,245
  • 9
  • 20