gianm commented on code in PR #20074:
URL: https://github.com/apache/druid/pull/20074#discussion_r3992125456


##########
processing/src/main/java/org/apache/druid/common/asyncresource/AsyncResources.java:
##########
@@ -98,24 +98,30 @@ public void onFailure(Throwable t)
    * Returns an {@link AsyncResource} backed by a {@link ListenableFuture} 
whose result <b>owns a lifecycle</b>: it
    * becomes ready when the future completes, exposing the result via {@link 
AsyncResource#get()}, and the result is
    * managed as a {@link Closeable}. Closing the resource closes the result, 
and a result that completes <i>after</i>
-   * the resource was already closed (a cancel/close-vs-completion race) is 
closed rather than leaked. Closing the
-   * returned resource before the future completes cancels the future ({@code 
cancel(true)}).
+   * the resource was already closed is closed rather than leaked.
+   *
+   * <p>Closing the returned resource deliberately does <b>not</b> cancel the 
future. Cancellation is what makes
+   * futures-of-closeables unsafe in the first place: a task that produces its 
value anyway hands it to a canceled
+   * future, which drops it silently, and nothing is left to close it. Leaving 
the future alone means the result
+   * always arrives through the callback below, which closes it when the 
resource is already gone. The cost is that
+   * work already submitted runs to completion; a producer that wants real 
cancellation should populate a
+   * {@link SettableAsyncResource} itself and give it a {@link 
SettableAsyncResource#setCanceler canceler} that can
+   * abort safely.
    *
    * <p>This is the managed counterpart of {@link #fromFutureUnmanaged}; use 
that for a future whose result is a plain
    * value or a completion signal with no lifecycle.
    */
   public static <T extends Closeable> AsyncResource<T> 
fromFutureCloseable(final ListenableFuture<T> future)
   {
     final SettableAsyncResource<T> retVal = new SettableAsyncResource<>();
-    retVal.setCanceler(() -> future.cancel(true));

Review Comment:
   👍 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to