Re: ThreadPoolExecutor - cancel rejected tasks with Discard*Policy?

An idea I tossed around for a while was to add chaining of RejectedExecutionHandler to DiscardPolicy and DiscardOldestPolicy.  
I never really spent the time to fully flesh out all of the problems so take it for what it is worth.
The idea is that only if the executor is shutdown then call the next RejectedExecutionHandler in the chain.  The caller would construct and specify the additional handler.
It would allow you to build things like DiscardOldest and CallerRuns or Discard and Abort.  If we added a cancel policy you could do Discard and Cancel.

Maybe we wouldn't have to deprecate if added chaining?

Jason

________________________________________
From: Concurrency-interest <[email protected]> on behalf of Martin Buchholz via Concurrency-interest <[email protected]>
Sent: Thursday, February 13, 2020 9:09 AM
To: Petr Janeček
Cc: concurrency-interest
Subject: Re: [concurrency-interest] ThreadPoolExecutor - cancel rejected tasks with Discard*Policy?

The Executor interface is simply
void execute(Runnable command)
which may have been a design mistake in retrospect (but it's analogous to Thread.run).
Giving a TODO to the executor should maybe require at least an acknowledgement in the form of a returned Future?
The default rejection policy is like DiscardPolicy, with "notification".
DiscardPolicy can be regarded as a "demo" Policy that might have been better confined to javadoc.
I agree with Doug that it may be safest today to soft-deprecate DiscardPolicy in the javadoc.

On Wed, Feb 12, 2020 at 2:03 AM Petr Janeček via Concurrency-interest <[email protected]<mailto:[email protected]>> wrote:
Hello,

I'm sorry to open a new thread on something I feel must have been discussed before,
I did not find any previous mention.

Imagine this:

```java
// Note the DiscardPolicy
ExecutorService executor = new ThreadPoolExecutor(
1, 1,
1, TimeUnit.MINUTES,
new ArrayBlockingQueue<>(1),
new ThreadPoolExecutor.DiscardPolicy()
);

Runnable task = () -> Uninterruptibles.sleepUninterruptibly(1, TimeUnit.SECONDS);
executor.submit(task);
executor.submit(task);
// The following task will get rejected and discarded by the pool:
executor.submit(task).get();
```

The code above will block forever, the `get()` call never returns. The task
had been rejected, but the returned Future is not cancelled, will never run,
and therefore will never be completed in any way. There's no way I'm aware
of to know the returned Future had been rejected.

This was very confusing and unexpected to me, I definitely expected the policy
to cancel the task. Note that this only happens with the Discard* policies
because the AbortPolicy throws and never returns a broken Future to the user,
while the CallerRunsPolicy returns a runnable Future.

I'd like to open a discussion around a few possible naive approaches to ease
the pain of future developers who get similarly caught by surprise:

1. Change the Discard*Policy to cancel the task. I did not find any place
where this would break any existing contracts. That said, obviously such
a change changes the behaviour in a significant way, and so it might not be
the way to go.

2. Introduce a DiscardAndCancelPolicy. Yes, writing such a policy is trivial
and anybody can do it if he needs it. The problem is that this is so obscure
that I cannot imagine many people do this right away. Discoverability is very
important here, and having an extra policy would tip people off.

3. Change the Discard*Policie's JavaDoc in a way it is clear that it does not
play with ExecutorService.submit() very well.

4. All of the above, or some more complex solution?

Thank you,
Petr Janeček
_______________________________________________
Concurrency-interest mailing list
[email protected]<mailto:[email protected]>
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
_______________________________________________
Concurrency-interest mailing list
[email protected]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest