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

"finally" for futures #6047

Open
benjaminp opened this issue May 12, 2022 · 1 comment
Open

"finally" for futures #6047

benjaminp opened this issue May 12, 2022 · 1 comment

Comments

@benjaminp
Copy link

benjaminp commented May 12, 2022

Consider the following simple snippet that does some work with a closeable resource:

Closeable c = allocateResource();
try {
  doWork(c);
} finally {
  c.close();
}

If we want to make doWork asynchronous, this code can be seemingly straightforwardly futurized with the help of addListener:

Closeable c = allocateResource();
ListenableFuture<Void> f = executor.submit(() -> doWork(c));
f.addListener(c::close, executor);

My concern with this translation is that if f is canceled, c.close may execute concurrently with doWork. That's a problem if c is not thread-safe. Ideally, c.close would be called when the task associated to f is not running, either because it finished or was preventing from even starting by cancellation.

I apologize if I've missed an existing pattern to deal with this situation. I looked at ClosingFuture—this issue is a sister of the one that prompted #3975—, but it seems that ClosingFuture is mostly sugar over the above addListener snippet and wouldn't prevent simultaneous execution of future tasks and closers during cancellation.

@netdpb
Copy link
Member

netdpb commented May 17, 2022

This makes me think that we should change ClosingFuture to guarantee that closing doesn't start until the future is really done, meaning on cancelation it would wait for any running task to complete. I don't know how feasible that is, though.

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

No branches or pull requests

4 participants