0

In other words, to my understanding, this is not thread safe (for multiple calls of startService()):

public class RaceService extends Service {

    volatile int a; // edit: added volatile to clarify my point

    @Override
    public int onStartCommand(Intent intent, int flags, int startId) {
        new Thread(new Runnable() {

            @Override
            public void run() {
                a++;
                Log.w("RaceService", "a: " + a);
            }
        }).start();
        return super.onStartCommand(intent, flags, startId);
    }
}

because there is only one instance of the service created (although never managed to find a clear assertion for this - if anyone could point me to the source where the service is actually instantiated (by reflection?) I would appreciate it). If one instance were created per startService() this would just print 1 on every call of startService.

But is this:

public class RaceService2 extends Service {

    int a;

    @Override
    public int onStartCommand(Intent intent, int flags, int startId) {
        a++;
        Log.w("RaceService2", "a: " + a);
        return super.onStartCommand(intent, flags, startId);
    }
}

thread safe (for multiple calls of startService()) ?

Would using an IntentService make a difference in these two examples (where the code would be in onHandleIntent()) ?

Mr_and_Mrs_D
  • 32,208
  • 39
  • 178
  • 361
  • Those callbacks are on the main thread. Background in this case does not mean async, it just means no UI – Greg Giacovelli Dec 28 '13 at 04:53
  • @GregGiacovelli: see edits – Mr_and_Mrs_D Dec 28 '13 at 16:42
  • 1
    Do you know what a race condition is? And if so what are you trying to prevent. Sorry your code does nothing useful really. It adds a number every time you send your service a command. Is this the real question? If you used an intent service, you would be on a background thread, but it is single threaded, so all intents would be handled serially but non blocking the main thread. So yes it still would be single threaded, just make sure to take care handling data between onCreate() and onHandleIntent() since they are on different threads – Greg Giacovelli Dec 28 '13 at 17:12
  • @GregGiacovelli: it does not add a number - it increments a number which is not an atomic operation and can easily lead to a race. Instead of `a++` you could have an arbitrary long computation. In code sample 1: race thradA reads a, threadB reads and increments and saves threadA increments and saves --> a==1. In sample 2 is there a chance the main thread executes onStartCommand when startService() is called two times in such a way that the same thing happens ? – Mr_and_Mrs_D Dec 28 '13 at 17:33
  • What I am saying is, of what consequence are you doing this all for? Logging? Then sure let it rip who the heck cares logs are not guaranteed to be sequential nor does it really matter then. It's the same as any other class where you use multiple threads. onHandleIntent is on somethreadA and all other callbacks are on `main` – Greg Giacovelli Dec 28 '13 at 17:34
  • @GregGiacovelli: my question is if there is a race. It is not a how but a why question. The example is just to illustrate – Mr_and_Mrs_D Dec 28 '13 at 17:41
  • I don't see the why anywhere in this question until your last comment. onStartCommand I believe is called on the main thread. So how does a single thread have a race condition with itself? – Greg Giacovelli Dec 28 '13 at 18:03
  • @GregGiacovelli: if the onStartCommand() calls are interleaved somehow. The mechanics behind startService() are not completelly clear to me. You might want to post an answer - threading is tricky – Mr_and_Mrs_D Dec 28 '13 at 18:07
  • thread1: onStartCommand() thread1: onStartCommand? Interleaved? How they are the same thread? @svenoaks is correct – Greg Giacovelli Dec 28 '13 at 18:10

1 Answers1

1

It's the same as the need for synchronization anywhere else in Java or Android - if you have multiple threads accessing the variables you may need synchronized methods to prevent thread interference or you may need to declare the variables as volatile so that each thread gets an up to date copy.

The only thing Android specific to note is that the onStartCommand is going to be called by the system on the main thread, so you are blocking the main UI thread unless you make it otherwise. If you use IntentService the onHandleIntent method is called on a different thread, so you can offload your long running task there without worrying about the mechanics of creating and using a separate thread.

A place where you might use a volatile instance variable would be if mutate a variable in onHandleIntent, and then access it in onDestroy. Here you want to ensure you have a fresh copy when onDestroy is called.

All that can be said of your examples is that in your first example, the value of a at the Log statement is undefined, and in the second example it is 1 if this is a new object.

EDIT: The bottom line is you really shouldn't be counting on the value of instance variables to be anything if you are calling startService() multiple times (we don't know if it's even going to be the same instance since it could have been destroyed) and you shouldn't count on the order of calls to startService() to necessarily be the same as the order they executed by the service (even though that seems to be the way it's implemented).

Stuff the requests into an array and send them as one call to startService(), or use local variables within the Service if you need such guarantees.

If you a more complicated scenario with multi-threading within the Service, you should should post a concrete example of the problem, because without knowing what you're trying to it's impossible to give advice.

Steve M
  • 9,296
  • 11
  • 49
  • 98
  • In the second example there is no race even for multiple calls of startService() ? – Mr_and_Mrs_D Dec 28 '13 at 06:15
  • Also the first example is a race because there is a single instance of the service class - but is this always the case ? – Mr_and_Mrs_D Dec 28 '13 at 06:20
  • I'm not sure what you mean, http://stackoverflow.com/questions/8019899/starting-a-service-multiple-times might be some help, there's only ever one instance of the Service, onStartCommand is always called on main thread, and onStartCommand can be called multiple times on the same object. – Steve M Dec 28 '13 at 06:23
  • Also I know all the facts in your second paragraph - but the question really is if the Intent service executes intents sequentially guaranteed - and if there is a memory barrier someplace which would synchronize between the worker and the main thread – Mr_and_Mrs_D Dec 28 '13 at 06:25
  • The only thing different about the first example is you don't know if the value of a will be incremented by the new Thread before or after the Log statement is executed. – Steve M Dec 28 '13 at 06:25
  • I don't know if calls to multiple startService to an IntentService are guaranteed to be added to the queue in the order they are called, I can't find any information. I would imagine so. What is known is that they are placed in a queue on a single worker thread and executed sequentially in onHandleIntent – Steve M Dec 28 '13 at 06:35
  • Look here, calls not documented to be in order: http://stackoverflow.com/questions/10030846/are-context-startservice-calls-guaranteed-to-be-aquired-by-the-service-in-the – Steve M Dec 28 '13 at 06:51
  • Look at my edits - and for IntentService I do not care about a specific order but about sequential guarantees. If this queue is concurrent I suspect it might act as a barrier - no ? – Mr_and_Mrs_D Dec 28 '13 at 16:52
  • 1
    One instance created means one is in existence at a time, the old one might be destroyed. In fact the instance is destroyed as soon as it's work is complete and there's nothing left in it's queue, I suggest trying to do a real world implementation and then post a new question if you have problems. – Steve M Dec 28 '13 at 17:06
  • If you could add some of @GregGiacovelli's comments and focus on the somewhat edited examples I might accept :) – Mr_and_Mrs_D Dec 28 '13 at 18:29
  • At this point, I doubt anyone knows what is being asked anymore. All the questions have been answered – Greg Giacovelli Dec 28 '13 at 19:05