Service doesn't stop itself about androidbillinglibrary HOT 15 CLOSED

Hey Richard.

Aren't Android services started with START_NOT_STICKY by default? There's not stopSelf() call, but the platform should get rid of the service if it needs to.

from androidbillinglibrary.

Hey, yes I think it is explicitly START_NOT_STICKY in the code, but I was thinking the service only needs to run as long as it is doing work. In another project I have used an IntentService to minimise the alive-time of the Service because they consume resources until RAM gets too low.

http://developer.android.com/reference/android/app/IntentService.html

In theory the BillingService could live for a very long time, and given it has also bound to the Market billing service via IPC it keeps that alive too. There's no real harm I suppose, but it's a bit strange for the Service to stay around after you've finish()ed the last Activity in the app?

Check this out:

http://www.androidguys.com/2009/09/09/diamonds-are-forever-services-are-not/

What do you think to this? Worth adding in some code to stopSelf() when the queue is done. We still have the service automatically started from the static methods and in reverse by the BroadcastReceiver, I don't think it'll suffer bad startup times.

from androidbillinglibrary.

Definitely keen on stopping the service if it's not necessary. We just tried to keep the first version as simple as possible.

What to give it a code at the code and send a pull request?

from androidbillinglibrary.

Sure, I will certainly give that a go. I'll post in this thread if I have any problem with GitHub (SVN user here).

from androidbillinglibrary.

Glad to help! We are also SVN users who are still learning our ropes with git.

from androidbillinglibrary.

I think it's just a case of adding:

stopSelf();

Under line 231 of BillingService.java. This machine is having some problems with GitHub's keys.

from androidbillinglibrary.

That might leave hanging pending requests. See mPendingRequests.

from androidbillinglibrary.

Ah yes sorry my actual code would be: if(mPendingRequests.size()<=1) stopSelf();

As it would be size 0 for immediate requests, and 1 when we're processing the last pending request? Hope I'm not missing something obvious here.

from androidbillinglibrary.

How do you know the request you're running is a pending request? It might not be, still have mPendingRequests with size 1, and the request waiting in the queue would be left hanging.

Sadly it's not as straightforward. One thing that could make it simpler is to consider all requests as pending requests.

The Dungeons example solved this problem, but the solution added a lot of code to the service that didn't have much to do with in-app billing.

from androidbillinglibrary.

I think that's a safer option too. We could check whether request == mPendingRequests.get(0) if the size() is 1, that way we know there's nothing more to come, but as you say, simpler modification is to just queue everything, even if it's immediately processed.

from androidbillinglibrary.

Yep. Sometimes micro-optimization gets in the way of clarity.

Do you still want to give it a go, or do you prefer us to tackle this in a future update?

from androidbillinglibrary.

I will try to work on that because it's an important issue because application never stops and system counts a lot of battery usage to the application. I found more an issue than a feature.

from androidbillinglibrary.

I made a pull request. I haven't tested extensively but it seems to work.

My implementation is trivial, I don't know if something escapes me. And I can't see the problems mentioned in older comments, because as there isn't any thread all runs in the same main UI thread and I don't see any advantage of the queued/unqueued requests.

from androidbillinglibrary.

this is an important issue, please consider lujop fix as soon as possibile.

an svn repository was mentioned, is it possibile to have its address?

from androidbillinglibrary.

By now I've that patch on production for one week and seems correct because I haven't any problems with it.
Some caution yet is needed because I'm not exactly using same codebase but there aren't important changes and I tried to apply correctly to trunk.

from androidbillinglibrary.