[External] : Re: Future.resultNow / exceptionNow

[Alex Otenko]

I view it the other way around. Since it is guarding resultNow, should
preclude all failures, which means FAILED or CANCELLED (we already know it
is not RUNNING).

Looking only for FAILED is a subtle bug.

Alex

On Sun, 21 Nov 2021, 18:10 Ron Pressler, <ron.pressler at oracle.com> wrote:

> I wrote precisely what I intended. You should only call exceptionNow if
> the state is FAILED (we might want to change the current behaviour). It’s
> just that this line shouldn’t be used in the context of the rest of your
> code. You should not need to poll or examine the state of futures at all
> when using StructuredExecutor, except in some very special circumstances:
> returning a partial result in the event of a timeout or cancellation.
>
> If you find yourself needing to test the state of the future, please show
> us when, as that’s something we’d like to avoid.
>
> — Ron
>
> On 21 Nov 2021, at 17:55, Alex Otenko <oleksandr.otenko at gmail.com> wrote:
>
> I think your code is also subtly incorrect, and because it is so easy to
> make this mistake people will get impaled on it all the time.
>
> Your example of the state check should be
>
> if (future.state() != SUCCESS) throw...
>
> Alex
>
>
> On Sun, 21 Nov 2021, 17:31 Ron Pressler, <ron.pressler at oracle.com> wrote:
>
>>
>>
>> > On 21 Nov 2021, at 15:57, Alex Otenko <oleksandr.otenko at gmail.com>
>> wrote:
>> >
>> >
>> > Sure. Here it is:
>> >
>> > exe.join();
>> > Throwable th = future.exceptionNow();
>> > if (th != null) throw rewrap(th);
>> > // here resultNow is guaranteed to be ok
>> > // but exceptionNow will throw
>> >
>>
>> I am not sure what you’re showing there. That code is obviously incorrect
>> — it asserts something that isn’t true -- and the error will tell you why.
>> If you want to do a check, why do you insist on using a method specifically
>> designed to do something else rather than use the new method intended for
>> that purpose? If you want to poll the state, do:
>>
>> If (future.state() == State.FAILED) throw rewrap(future.exceptionNow());
>>
>> >
>> > People will use it as a get() that doesn't require try-catch.
>>
>> resultNow can only be used in a state where it is the right thing to do.
>>
>> >
>> > Well, and my contention is that you declare it a bug. What's the
>> horrible sin of the above check? A throwing exceptionNow doesn't really
>> save me from anything.
>>
>> The sin is the same one as in calling Lock.unlock without holding the
>> lock. resultNow is a method that says: I know that this future has a
>> result, so give it to me. If you do "I know that this future has a result
>> so give it to me” but it doesn’t have a result, then you’re using this
>> operation incorrectly.
>>
>> We’ve added a method that allows you to do what you want. Why do you want
>> to do that thing not with the method we’ve added for that purpose but with
>> the method we’ve added for a different purpose?
>>
>> — Ron
>
>
>