Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition with GC can result in callbacks never being called #453

Closed
Aaargh20318 opened this issue Feb 18, 2014 · 3 comments
Closed
Assignees
Labels
Milestone

Comments

@Aaargh20318
Copy link

AsyncHttpResponseHandler.ResponderHandler uses a WeakReference to AsyncHttpResponseHandler. When using the library as documented using a anonymous response handler, a race condition occurs between the garbage collector and AsyncHttpResponseHandler.ResponderHandler

In this scenario only AsyncHttpRequest keeps a strong reference to the AsyncHttpResponseHandler, at the end of it's run method it sends a message through the handler and then the run methods finishes. At this point the AsyncHttpRequest is garbage and since it is the only one holding a strong reference to it, so is the AsyncHttpResponseHandler. If the GC runs between the run method finishing and handleMessage in AsyncHttpResponseHandler.ResponderHandler retrieving the AsyncHttpResponseHandler from the WeakReference (you can simulate this by inserting a System.gc at the top of handleMessage) the weak reference is null and handleMessage cannot be called.

The result of this is that in some scenario's a HTTP request never gets a callback. neither a failure nor a success. This occured when I was using the lib to load a lot of images on a fast network connection, some never succeeded or failed so no retry was performed either.

@smarek
Copy link
Member

smarek commented Feb 18, 2014

Hi @Aaargh20318, this is long known issue, and it should be fixed by merge ( 5ee354d ) done today, can you please verify ? I've got no time to deeply analyse memory model now.
Thank you anyway for your analysis, it's greatly appreciated. 👍

@smarek smarek self-assigned this Feb 18, 2014
@smarek smarek added this to the 1.4.5 milestone Feb 18, 2014
@Aaargh20318
Copy link
Author

Thanks, great timing for that merge :) I was just trying to figure out the best approach to fixing this.

Looks like this solves the issues I've been seeing.

@smarek
Copy link
Member

smarek commented Feb 18, 2014

Great, closing the issue, thank you

@smarek smarek closed this as completed Feb 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants