Android thread in service issue

Question

I am having a remote service running in which i am spawning a new thread to keep polling for incoming messages via socket and then at certain intervals i want to stop the socket and restart later...stopping is working fine (stopUARTConnection()) but re starting(startUARTConnection) is not causing the while loop again.. Can someone tell me whats wrong ?

public class BackgroundService extends Service{

ServerSocket server = null;
Socket socketClient = null;
InputStream is = null;
BufferedReader br = null;
char[] buff = null;
boolean threadRunning = true;

private static final String TAG = BackgroundService.class.getSimpleName();
private Thread socketThread = null;
int temp = 0;

@Override
public IBinder onBind(Intent intent) {
    if (BackgroundService.class.getName().equals(intent.getAction())) {
        Log.d(TAG, "Bound by intent " + intent);
        return apiEndpoint;
    } else {
        return null;
    }
}

private final Object latestIncomingMessage = new Object();
private List<MessageCollectorListener> listeners = new ArrayList<MessageCollectorListener>();
private Message message = new Message(" ");

private MessageCollectorApi.Stub apiEndpoint = new MessageCollectorApi.Stub() {
    @Override
    public void removeListener(MessageCollectorListener listener) throws RemoteException {
        synchronized (listener) {
            listeners.remove(listener);
        }
    }

    @Override
    public Message getValue() throws RemoteException {
        synchronized (latestIncomingMessage) {
            return message;
        }
    }

    @Override
    public void addListener(MessageCollectorListener listener) throws RemoteException {
        synchronized (listener) {
            listeners.add(listener);
        }
    }

    @Override
    public void startBroadCastConn() throws RemoteException {
        try {
            startUARTConnection();
        } catch (IOException e) {
            Log.d("B Service", "Stop UART CONNECTION");
        }

    }

    @Override
    public void stopBroadCastConn() throws RemoteException {
        try {
            stopUARTConnection();
        } catch (IOException e) {
            Log.d("B Service", "Stop UART CONNECTION");
        }           
    }
};

public boolean createSocket(int port) {
    try{
        server = new ServerSocket(port);
    } catch (IOException e) {
        return false;
    }
    return true;
}

public boolean listenSocket(){
    try{
        socketClient = server.accept();
    } catch (IOException e) {
        return false;
    }
    return true;
}

@Override
public void onCreate() {
    super.onCreate();
    socketThread = new Thread(){
        @Override
        public void run() {
            while(threadRunning){
                boolean socketCreated = createSocket(8080);
                boolean socketListen = listenSocket();
                if(socketListen == true && socketCreated == true){
                    //To add code for processing data..
                    threadRunning = true;
                }
                try {
                    recycle();
                } catch (IOException e) {
                    //e.printStackTrace();
                }
            }
        }   
    };
    socketThread.start();
}
public void stopUARTConnection() throws IOException{
    recycle();
    threadRunning = false;
}

public void startUARTConnection() throws IOException{
    recycle();
    threadRunning = true;
}

private void recycle() throws IOException{
    if(socketClient != null){
        socketClient.close();
        socketClient = null;
    }   
    if(is != null)
        is.close();
    if(server != null){
        server.close();
        server = null;
    }
}

@Override
public void onDestroy() {
    super.onDestroy();
    try {
        recycle();
    } catch (IOException e) {
        Log.d("B Service", "Failed to recycle all values");
    }
    socketThread = null;
}

}

Vincent Mimoun-Prat · Accepted Answer · 2011-04-06T09:26:15.067

2

You are creating the Thread only once. The loop is thus run only once until it is instructed to exit.

public void startUARTConnection() throws IOException{
    recycle();
    socketThread = new SocketThread();
    socketThread.start();
}

public void stopUARTConnection() throws IOException{
    recycle();
    socketThread = null;
}

private class SocketThread extends Thread {
    @Override
    public void run() {
        // Loop until socketThread is assigned something else 
        // (a new Thread instance or simply null)
        while (socketThread==this){
            boolean socketCreated = createSocket(8080);
            boolean socketListen = listenSocket();
            if(socketListen == true && socketCreated == true){
                // To add code for processing data..
            }
            try {
                recycle();
            } catch (IOException e) {
                //e.printStackTrace();
            }
        }
    }   
};

edited Apr 06 '11 at 09:26

answered Apr 06 '11 at 09:21

Vincent Mimoun-Prat

28,208
16
81
124

hmmm... looks right...but should nt we destroy the thread before starting again...should i just assign the thread to null to do it? – Arun Abraham Apr 06 '11 at 09:30
`socketThread==this` will detect that you have created a new SocketThread and the old Thread will then exit. If you need to be sure you don't have 2 threads running at the same time, you'll need to implement a check based on `join()`. – Vincent Mimoun-Prat Apr 06 '11 at 09:33
Hey....thanks a lot...your solution was right..thanks a lot..really new to threads... :) – Arun Abraham Apr 06 '11 at 09:45
@Arun Abraham: If Marvin has solved your problem, you really should accept the answer... – forsvarir Apr 06 '11 at 10:10

score 0 · Answer 2 · edited May 23 '17 at 12:19

0

When you call stopUARTConnection, you're setting threadRunning=false... at this point, your while loop is exiting... in startUARTConnection, you're setting threadRunning to true, but you're not restarting the thread.

As @Marvin has pointed out, you can't just call start on the thread object, you need to recreate it.

edited May 23 '17 at 12:19

Community

1
1

answered Apr 06 '11 at 09:15

forsvarir

10,749
6
46
77

-1 because you cannot start a Thread that has already been started once. Will throw IllegalThreadStateException the second time startUARTConnection is called. – Vincent Mimoun-Prat Apr 06 '11 at 09:22
hear i have to destroy that thread before doing start again..can we destroy by just assigning the thread = null; ??? – Arun Abraham Apr 06 '11 at 09:26
@Arun Abraham: see Marvin's response. You shouldn't have to destroy the thread (it will have exited), you can create a new one over the existing reference – forsvarir Apr 06 '11 at 09:28
@MarvinLabs: Thanks for the pointer. I wasn't aware that you couldn't restart a threadobject, I assumed one it entered an exit-state it would be ok. I guess it's not something I've ever needed to do, typically if I start a thread I'm not expecting to shut it down until the application is ready to exit. – forsvarir Apr 06 '11 at 09:32
Always explicitly shutdown the threads you start, else they will just keep running in the background and you'll leak memory... – Vincent Mimoun-Prat Apr 06 '11 at 09:36
@MarvinLabs: Doesn't it shutdown once it exits the run method? – forsvarir Apr 06 '11 at 09:37
It does shutdown **if** it exits the run method (lots of people have a loop that will not exit on its own but needs to be told to exit explicitly) – Vincent Mimoun-Prat Apr 06 '11 at 09:40

score 0 · Answer 3 · answered Apr 06 '11 at 09:20

0

I think it is better to use a TimerTask instead of thread/while and ConditionVariable instead boolean variable threadRunning

answered Apr 06 '11 at 09:20

Mithun Sreedharan

49,883
70
181
236

Android thread in service issue

3 Answers3

Linked