Skip to content
This repository has been archived by the owner on Jan 14, 2018. It is now read-only.

Request dropped when stopping the SpiceManager #246

Closed
doridori opened this issue Jan 10, 2014 · 18 comments
Closed

Request dropped when stopping the SpiceManager #246

doridori opened this issue Jan 10, 2014 · 18 comments
Assignees
Milestone

Comments

@doridori
Copy link

Hi.

I have encountered the issue referred to by @stephanenicolas on #143

I am calling SpiceManager.shouldStop() directly after calling SpiceManager.execute(...) - which as you say results in the request not being executed. I think this is a design flaw as the SpiceService should only really stop when all activities / fragments are unbound and all requests have finished executing. This is a viable use-case IMHO as I want to use the same thread-pool as my other SpiceRequests but in this case I do not need to wait for the request to return before leaving the activity.

Am I missing something here?

Love Robospice apart from that :) Thank you

@ghost ghost assigned stephanenicolas Jan 11, 2014
@stephanenicolas
Copy link
Owner

I will try to do my best to fully support "Fire and Forget" requests. I also consider it a very valid use case but didn't find much time to work on it yet. I will tell you when I start working on that issue. Meanwhile, if you have any idea or pull request, let me know.

I think the main thing is to get sure all pending requests inside the SpiceManager are actually sent to the spice service before it gets detroyed and wipes out every listener. I think it should not be a long patch, but we have to precisely identify where to patch.

Thx for reporting the issue and the nice words on the lib.

@stephanenicolas
Copy link
Owner

Hi @doridori , I think I found the bug and could patch it. That's a very cool bug indeed. I think it will make our tests much more robusts in all RS. Thanks for reporting it.

Nevertheless, I adopted a very pragmatic bug resolution approach. I added a test, and everything works fine. My fear is to have created a very tiny bottleneck on the UI thread in some cases. Would you mind to test the snapshots of RS (1.4.11)? They will be available within 10 minutes.

@stephanenicolas
Copy link
Owner

Thread is closed, reopen if needed.

@doridori
Copy link
Author

Hi @stephanenicolas! thanks for looking at this

I have just tested by replacing the 1.4.9 jars with the latest 1.4.11s taken from https://oss.sonatype.org/content/repositories/snapshots/com/octo/android/robospice/

robospice-1.4.11-20140113.140209-5.jar
robospice-cache-1.4.11-20140113.140107-5.jar
robospice-okhttp-1.4.11-20140113.140734-3.jar

and still the same issue. loadDataFromNetwork never seems to be called (for the use case described above) :(

@stephanenicolas
Copy link
Owner

Really strange, I use your sample, and get it working : I see the toast in both landscape and portrait when I use the patch.

@doridori
Copy link
Author

Just rebuilt my whole project in case the old jars were cached - still not working...! its not a big issue for me as I have moved the call but strange - i dont have much time to look into atm

@stephanenicolas
Copy link
Owner

What version of android are you using ?

It really bugs me.

2014/1/13 Dorian Cussen [email protected]

Just rebuilt my whole project in case the old jars were cached - still not
working...!


Reply to this email directly or view it on GitHubhttps://github.com//issues/246#issuecomment-32176699
.

Stéphane NICOLAS,
OCTO Technology
Développeur & Consultant Android / Java
..........................................................
50, Avenue des Champs-Elysées
75008 Paris
+33 (0)6.26.32.34.09
www.octo.com - mobilite.octo.com
blog.octo.com - www.usievents.com
...........................................................

@doridori
Copy link
Author

4.4.2, Nexus 4 :)

@stephanenicolas
Copy link
Owner

Same here...

2014/1/13 Dorian Cussen [email protected]

4.4.2, Nexus 4 :)


Reply to this email directly or view it on GitHubhttps://github.com//issues/246#issuecomment-32178219
.

Stéphane NICOLAS,
OCTO Technology
Développeur & Consultant Android / Java
..........................................................
50, Avenue des Champs-Elysées
75008 Paris
+33 (0)6.26.32.34.09
www.octo.com - mobilite.octo.com
blog.octo.com - www.usievents.com
...........................................................

@doridori
Copy link
Author

I am executing with mSpiceManager.execute(new AppConfigCall(), null); and avoiding the cache, in case that handled differntly internally

@doridori
Copy link
Author

I have tried recreating the issue in a small activity and I cant do it - loadDataOnNetwork() is called each time. I stupidly am making the call before onStart (triggered by something in onCreate) so i thought that was the issue - but in my test activity this works just fine also...sorry - not very helpful

@stephanenicolas
Copy link
Owner

@doridori , ok I got a new build for you. Do you mind to test this new SNAPSHOT ?
It will be available within 15 mn.

stephanenicolas added a commit that referenced this issue Jan 14, 2014
@doridori
Copy link
Author

sure will do later today :)

@stephanenicolas
Copy link
Owner

That's the last open issue before releasing. The better you can test, the better this release will be. :)

@doridori
Copy link
Author

as i mentioned before i am making the call in onCreate() before finish() is called. If this happens newer android versions will not call Activity.onStart() and onStop() - so the SpiceManager was never started, this was not intended. However - when the SpiceManger is started in the next activity the request has been lost. Sorry I shouldve seen this before - its only in a subset of use-cases so was not immediatly obvious!!

This does still happen with the latest snapshots from earlier

@stephanenicolas
Copy link
Owner

@doridori

Ok, so did I solve your non bug ? (And then we can go for a release ?)

Indeed, there was a real problem for "fire and forget" requests : you execute a request, let's say in onStart, and then finish the activity. In such a case, requests would have been lost and they are not anymore. You won't be notified of the result (as the activity and its listeners died), but the request will still be executed.

Actually, since that very subtile change, it looks like all tests are much much more stable than ever before. I can even re-run parallel maven builds when building RoboSpice, and this is really a great sign of quality increase according to me.

@doridori
Copy link
Author

Yes i think so :)

Good work! Many thanks

@stephanenicolas
Copy link
Owner

Thanks to you too. It really helped.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants