Reputation: 14571
Scenario
Step 1: Init the location manager to read GPS locations every 50 meters:
locationManager.requestLocationUpdates(LocationManager.GPS_PROVIDER, 0, 50, locationListenerGps);
Step 2: each time a location is read:
@Override
public void onLocationChanged(Location location) {
new Thread(new Runnable() {
@Override
public void run() {
sendLocation(location);
}
}).start();
}
Step 3: on sendLocation there are a few things I do:
The problem
All this is done in background in a service. For each sendLocation call I make a new thread. While connectivity is ok, everything works fine. But, when sending fails and the user is driving, the location read happens very often and there are big chances that there are 2-3 threads all trying to send the same unsent locations. If Thread1 receives the list and tries to send it, Thread2 and Thread3 should not be able to read it and try to send it as Thread1 may send it successfully. How can I prevent this from happening ? How can I make sure Thread2 does not read the list ?
From what I am thinking now, I could add a new field in the table "processing" and for all the rows retrieved for sending, update the field to true. In this case Thread2 will only get the processing=false rows. Is this a solution ? Any other recommendations ? I still believe that there is a slight change for Thread2 to get the data, while Thread1 is updating processing... Thanks.
Later edit: Extra thoughts and ideas I have tried this approach
private ExecutorService threadPool;
@Override
public void onCreate() {
scheduleTaskExecutor = Executors.newSingleThreadScheduledExecutor();
threadPool = Executors.newSingleThreadExecutor();
//also need to send location every 60 seconds if no other location was read
scheduleTaskExecutor.scheduleAtFixedRate(new Runnable() {
@Override
public void run() {
sendLocation(lastLocation);
}
}, 60, 60, TimeUnit.SECONDS);
}
@Override
public void onLocationChanged(Location location) {
threadPool.execute(new Runnable() {
@Override
public void run() {
sendLocation(location);
}
});
}
@Override
public void onDestroy() {
threadPool.shutdownNow();
}
From what I read, this threadPool should force threads to execute one after another, right ? (even I do have a feeling I misunderstood its purpose) If so, what happens if I get no connectivity for an hour ? For each location read, a new thread is added... but how long does this thread last ? I am concern what happens if the user is driving really fast, I could get locations read every 1-2 seconds, would this mechanism hold my web access in a queue, one thread after another ?
In another order of thoughts, what if onCreate method of the service I make a new thread. Something like:
@Override
public void onCreate() {
new Thread(new Runnable() {
@Override
public void run() {
startLocationListener();
}
}).start();
}
and in startLocationListener() I start GPS location read. Will onLocationChanged be executed on this thread and won't interfere with my UI thread ?
Would it be wiser to use a Service that runs in its own thread ? So I won't have to worry about threading ?
Using the current approach, the app does the job but there is something wrong happening, randomly and can't figure out the reason: one of my activities binds to the service to receive updates, I carefully unbind it when the app gets onPause... but sometimes the service keeps running, as I can see its notification icon displayed. I will investigate this more, but I need to settle a strong/reliable way of handling location reading and sending.
Later later edit
How about this approach:
private ExecutorService scheduleTaskExecutor;
@Override
public void onCreate() {
scheduleTaskExecutor = Executors.newSingleThreadScheduledExecutor();
//also need to send location every 60 seconds if no other location was read
scheduleTaskExecutor.scheduleAtFixedRate(new Runnable() {
@Override
public void run() {
sendLocation(lastLocation);
}
}, 60, 60, TimeUnit.SECONDS);
}
@Override
public void onLocationChanged(Location location) {
scheduleTaskExecutor.submit(new Runnable() {
@Override
public void run() {
sendLocation(location);
}
});
}
@Override
public void onDestroy() {
scheduleTaskExecutor.shutdownNow();
}
Upvotes: 2
Views: 525
Reputation: 62439
So let me get this straight: you want to send locations one after the other from a background thread. A simple scheme to achieve this would be (similar to your edited code, but I don't see the reason for ScheduledExecutor
):
private ExecutorService exec;
@Override
public void onCreate() {
exec = Executors.newSingleThreadExecutor();
}
@Override
public void onLocationChanged(Location location) {
exec.submit(new Runnable() {
@Override
public void run() {
sendLocation(location);
}
});
}
@Override
public void onDestroy() {
exec.shutdownNow();
}
What this does under the hood is basically to create a background thread and a queue of tasks. Every time a location is read a new task is put into the queue. The thread continuously polls the queue and executes the tasks in order.
Upvotes: 1
Reputation: 1006674
For each sendLocation call I make a new thread.
Why?
But, when sending fails and the user is driving, the location read happens very often and there are big chances that there are 2-3 threads all trying to send the same unsent locations.
This is why I asked "Why?" above.
If Thread1 receives the list and tries to send it, Thread2 and Thread3 should not be able to read it and try to send it as Thread1 may send it successfully. How can I prevent this from happening ? How can I make sure Thread2 does not read the list ?
IMHO, by not having Thread2 and Thread3 in the first place. Use a single thread at a time, that sends all unsent data. That is probably a long-lived thread, working off of a work queue (coupled with some sort of timer mechanism to handle the case where you failed to update before and wish to make sure you try again after X period of time, if no other events forced you to try sooner than that). I don't see why you would need more than that to achieve your aims.
Upvotes: 1