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

Future isn't interrupted when RequestFuture is cancelled #143

Closed
blinkmacalahan opened this issue Feb 9, 2018 · 3 comments
Closed

Future isn't interrupted when RequestFuture is cancelled #143

blinkmacalahan opened this issue Feb 9, 2018 · 3 comments

Comments

@blinkmacalahan
Copy link

I've been digging relatively deep into the code for about a day and it appears that cancelling the RequestFuture only cancels the request and not the Future itself. I would expect when cancelling the RequestFuture, the associated Future would throw a InterruptedException. That way my blocking code would stop blocking and move on. I've tried following what the documentation says and have set my request on the future, but an InterruptedException is never thrown. Instead, I have to wait for the TimeoutException occur which isn't ideal.

 RequestFuture<JSONObject> future = RequestFuture.newFuture();
 MyRequest request = new MyRequest(URL, future, future);

 // If you want to be able to cancel the request:
 future.setRequest(requestQueue.add(request));

try {
   JSONObject response = future.get(50,
                    TimeUnit.SECONDS);
   // do something with response
 } catch (InterruptedException e) {
   // handle the error
 } catch (ExecutionException e) {
   // handle the error
 }

As I mentioned I've been digging into the code a while but my apologies if I'm missing something. Thanks!

@jpd236
Copy link
Collaborator

jpd236 commented Feb 9, 2018

Sorry that you ended up spending a day on this... unfortunately it's a known issue (#85). Fixing it is unfortunately non-trivial as noted there.

@jpd236 jpd236 closed this as completed Feb 9, 2018
@blinkmacalahan
Copy link
Author

I did see that issue (#85) but felt it was different than what I'm suggesting. I feel like adding a Cancel listener is completely different than expecting the Future itself to get canceled. However, possibly the work to do both is related so I'll leave it to your discretion.

As a future FYI to others that might stumble across this thread, an extremely hacky way to work-around this issue is if the request is already in flight, when you go to cancel your future, you can also return null to the Future's onResponse. Make sure your code handling the response looks for null and handles it as a failure.

@jpd236
Copy link
Collaborator

jpd236 commented Feb 9, 2018

The issue is that RequestFuture (which is just a helper class in the .toolbox package) needs to know when the request was canceled in order to wake up the sleeping monitor in get() - that's what the cancel listener would be used for.

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

No branches or pull requests

2 participants