Canceling executor during uploadLargeFile() makes it hang.
Marj did some great work and found a cool bug. :)
Currently, if you want to stop a large file upload, you need to call shutdown on the executor you provided. Unfortunately, if there are still tasks that haven't started uploading yet, calling shutdown returns them to the code that called shutdown and uploadLargeFile() ends up waiting in a call to get() on the corresponding future.
Thanks for tracking this down Marj!
Here's a stackoverflow question on this topic that Marj found:
https://stackoverflow.com/questions/27254869/future-get-hangs-when-callback-catches-interruptedexception
We haven't figured out a way to get notified that the executor is shutdown.
We've discussed a few options...
(1) changing the future.get() calls to have a timeout and, if the timeout fires, check for the executor having been shutdown. this is basically polling. i has the benefit of not changing the API.
(2) providing a callback object that the uploader can use to ask "should i keep uploading?" callers to uploadLargeFile would be able to start returning false to cause a shutdown. this is another form of polling.
(3) making a new startLargeFileUpload() that returns a future, so that callers can use get() to wait for completion or call cancel() to ask for a clean cancelation of just that one upload (without having to stop the whole executor).
i think i prefer (3). while it changes the API, it's the cleanest and it's not polling. AND, the existing method can be reimplemented in terms of the new api. on the downside, we'd still have to tell people to not shutdown an executor until they're sure it's not in the middle of uploading.
as far as we know, this is a corner case and not actively hurting anyone, so we will think about it before rushing to make a fix.
thoughts?