Structured concurrency: TaskHandle

[forax at univ-mlv.fr]

----- Original Message -----
> From: "attila kelemen85" <attila.kelemen85 at gmail.com>
> To: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "Ron Pressler" <ron.pressler at oracle.com>, "Alan Bateman" <alan.bateman at oracle.com>, "loom-dev"
> <loom-dev at openjdk.org>
> Sent: Friday, May 12, 2023 10:05:42 PM
> Subject: Re: Structured concurrency: TaskHandle

> I really love Rémi's proposal about merging the `state`, `get` and
> `exception` into a single call, otherwise it will push people towards
> creative ways to check the result state creating multiple patterns for
> it. While Rémi's sealed interface proposal just cannot be
> misinterpreted. For example, consider the documentation of
> `exception`: "Returns the exception, if the task failed with an
> exception." It is not exactly obvious what if the task failed with an
> exception due to a cancellation? In case shutdown was called before
> the task completed the current proposal requires the cancelled state.
> So, is that exception lost now? That seems to me a problem, because it
> is perfectly possible that due to one task's action (done due to
> cancellation) another went into a bad state, and you do want to make
> it visible.

yes, the exception is not lost, the task state is FAILED and exception() returns the exception thrown by the Callable due to InterruptedException been thrown by a blocking call.

> 
> ### Cancellation
> 
> Cancellation is important enough to have very special care in my
> opinion. In fact - to me - it was so important that I wrote a brand
> new executor framework for my usage and happily using that instead of
> the built-in for 10+ years now.
> 
> The question that should be asked is what are the important cases for
> cancellation?
> 
> 1. The task did not even start. This is important to know, because it
> is very easy to argue about the consequences of this case. Though not
> completely free
> 2. The task did not complete before `shutdown`, but failed with a
> specially interpreted type of exception.
> 3. The task did not complete before `shutdown` was called, but started
> execution, and threw an unexpected exception.
> 4. The task did not complete before `shutdown` was called, but started
> execution, and returned without an exception.
> 
> I think only (1) and (2) should be marked as cancelled, and this also
> implies that - in Rémi's sealed interface proposal - the `Canceled`
> result should have an nullable `Throwable` field.

yes, that why i was proposing to lazily allocate an InterruptedException instead of returning null.
With that, from a user POV, calling shutdown() always result in a failed task with exception() returning an InterruptedException.

[...]

> 
> 
> ### TaskHandle.task()
> 
> This method is a menace. This was one of the many cancellation problem
> of the JDK's thread pool (though was not restricted to make this
> mistake by its interface): I often needed the case (especially for UI)
> where I bombarded an executor with tasks, but the tasks were
> idempotent, so there is no reason to have more of them queued. Yet,
> the cancellation of the JDK's thread pool did not remove the canceled
> task from its queue upon cancellation retaining a reference to the
> task object. This can be quite the issue, and this method on
> `TaskHandle` will now force the JDK to always keep a reference to the
> task, even though it is largely useless. If you really need it, you
> have the option to retain it and associate it with the `TaskHandle`.
> So, my opinion is that this must be removed in order to allow the GC
> to reclaim the task and whatever potentially large object it is
> referencing.
> 
> If we really want to provide this for the STS (because that is the
> only place which would not need about it otherwise), then that could
> be provided in a separate argument, and that would also allow the GC
> to reclaim the task.

I agree, keeping the callable too long often leads to hard to diagnose memory leak.

> 
> Attila

Rémi