> public void addToClose(Closeable toClose) {
> methodsToClose.add(toClose);
> }
>
> public void close() throws IOException {
> - Collections.reverse(methodsToClose);
> - for (Closeable toClose : methodsToClose) {
> - toClose.close();
> + if (state.compareAndSet(State.OPEN, State.CLOSING)) {
IMO this change is OK. I can't come up with a use case where closing the
context multiple times would be needed. The main things to be closed in the
context are the executor services, and once they are shutdown I don't see the
point in shutting down them again.
Also, once a context is closed, there are no means to "reopen" it, apart from
creating a new one, so I think this won't break any existing code (unless
someone is adding arbitrary stuff to the Closer once the context is created,
which I doubt).
Having said this, would it be a good idea to try/catch each call to
`toClose.close()` so even if a Closebale can't be closed we try to close the
others? Closing the context should be the very last operation when using
jclouds. I think it should be good to try to close everything and just leave a
log with the close failures.
Anyway, this PR looks good to me and if you @demobox agree, I think it can be
merged once the `isOpen()` comment is addressed, and we can open the try/catch
thing as a separate issue.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/32/files#r4723718
