[concurrency-interest] Proper workaround for FutureTask.set()/get() race (CR 7132378)

Joe Bowbeer joe.bowbeer at gmail.com
Mon Apr 23 18:10:38 EDT 2012


For library maintainers, consistency with the existing API is also of
importance.  Each new method is not added in isolation.

In this case, the closest comparables (like real estate?) are:

1. Future.cancel(mayInterrupt).  Implemented by FutureValue.  This is a
best-effort method whose name is not prefixed with "try".  Returns boolean;
does not throw exception.

2. FutureTask.set(V) and FutureTask.setException(T).  These are protected
methods in FutureValue's sibling class FutureTask.  These methods have no
effect if the FutureTask has already completed.  Return nothing; do not
throw exception.

If FV's setters do not behave the same as FT's, then I see some
justification for naming them differently, even though FT's setters are
"protected" and not "public".

--Joe

On Mon, Apr 23, 2012 at 2:33 PM, Gregg Wonderly wrote:

> One of the things that I think is important to consider for any software
> system, is that partial failure is always something to consider.  In the
> days of multi-programming, we counted partial failure as a "major failure"
> because typically a whole program/process disappeared or otherwise failed
> to complete. In this day and age, and in this forum in particular, we are
> using multi-threading as the preferred programming practice.
>
> Partial failure is still a vital system design issue.  You need to think
> about how your system will either fail, or complete/retry something that
> did not "complete" as the system expected it to.
>
> Throwing checked exceptions, for me, is something that one should always
> consider as a way to say, there is no choice to ignore this issue.
>
> Boolean returns or null/non-null returns etc, are for me, less partial
> failure oriented and more logic-processing oriented.
>
> If, in this API, setting twice, is a programmatic error, than an exception
> should be thrown.  If it is a logic flow management technique, than that's
> fine, but the API should help the developer recognize how the API works by
> using meaningful method names.
>
> For me, set(), is not meaningful in this case.  There needs to be some
> "please attempt to set this value and tell me how that works out" method
> name, if the API design truly needs to "allow" the developer the choice of
> using failure-to-set as a logic flow mechanism.
>
> Gregg Wonderly
>
>
> On 4/23/2012 3:07 PM, Ariel Weisberg wrote:
>
>> Hi,
>>
>> I like try as well. Throwing as the default behavior results in
>> try/catch noise and auto-generated catch clauses that people forget to
>> fill in.
>>
>> It's helpful for people who are new to concurrency to use APIs that are
>> explicit about what kind of error conditions they should be looking for.
>> A separate method with a checked exception outranks a Javadoc note of a
>> return value in terms of visibility, and it is harder for a second
>> author
>> to come along and remove the check during a rewrite.
>>
>> There are several collections that offer versions of methods that throw
>> NoSuchElementException. I don't see this is as being very different.
>>
>> Ariel
>> On Mon, Apr 23, 2012, at 01:33 PM, David M. Lloyd wrote:
>>
>>> Sounds reasonable.  A "try..." prefix is also an option for this kind
>>> of thing.
>>>
>>> On 04/23/2012 01:22 PM, Gregg Wonderly wrote:
>>>
>>>> For me, the "putIfAbsent" name, says that you care to ignore
>>>> failures. "set" doesn't indicate a desire to ignore failures. Not
>>>> checking the true/false return will be a point of "failure". If an
>>>> exception is thrown on failure, than it tells the developer, quite
>>>> directly, that there is an expected usage pattern.
>>>>
>>>> If both patterns need to be supported, than make the API support
>>>> both literally.
>>>>
>>>> public boolean setIfNotSet( T val ); public void set( T val ) throws
>>>> ValueAlreadySetException;
>>>>
>>>> The boolean return feels more like a CAS operation. For some people,
>>>> CAS operations are "friendly". For others who don't use those kinds
>>>> of APIs, and don't understand races in their code, it can be quiet a
>>>> bit painful to see that when you say "set" like so many other
>>>> "property" like functions, it doesn't actually "set" the value.
>>>>
>>>> Don't use "set" as the API if it will return boolean. Pick something
>>>> more verbose so that it is clear that the user needs to check the
>>>> return value.
>>>>
>>>> Gregg Wonderly
>>>>
>>>> On 4/23/2012 12:45 PM, David M. Lloyd wrote:
>>>>
>>>>> To put it another way - I have extensively used Future-like objects
>>>>> which have three result methods: setResult(T result),
>>>>> setFailure(Exception cause), setCancelled(), each returning a
>>>>> boolean. The first one called returns "true", the latter ones
>>>>> return "false". This hugely simplifies asynchronous cancellation
>>>>> logic. Yes there are non-sane invocation possibilities but the
>>>>> results are always deterministic, and it works pretty well in
>>>>> practice.
>>>>>
>>>>> On 04/23/2012 12:34 PM, Joe Bowbeer wrote:
>>>>>
>>>>>> I disagree as well. I use futures a lot (and in addition to using
>>>>>> FindBugs, I write my own custom FindBugs detectors:-)
>>>>>>
>>>>>> For me, ignoring duplicates is better than throwing an exception.
>>>>>>
>>>>>> The simplest use case is when the value is canceled by the owner
>>>>>> and then subsequently set by a worker. Cancel wins.
>>>>>>
>>>>>> Or when the value is farmed out to several competing strategies,
>>>>>> and first one to answer wins.
>>>>>>
>>>>>> The boolean result of the set methods is symmetric with the
>>>>>> boolean result of cancel.
>>>>>>
>>>>>> --Joe
>>>>>>
>>>>>> On Mon, Apr 23, 2012 at 10:10 AM, David M. Lloyd wrote:
>>>>>>
>>>>>> I disagree. May times using set() on an object like this is the
>>>>>> point of coordination. Would you throw an exception if
>>>>>> putIfAbsent() failed?
>>>>>>
>>>>>>
>>>>>> On 04/23/2012 11:37 AM, Nathan Reynolds wrote:
>>>>>>
>>>>>> Consider throwing an exception instead of returning false. The
>>>>>> exception will force the caller to deal with the already set
>>>>>> condition. Returning false is a bug waiting to happen.
>>>>>>
>>>>>> Calling set() twice usually indicates a larger bug in the caller
>>>>>> code. Why would the algorithm call it twice? Is there a race
>>>>>> between 2 threads in the caller code? Is one part of the caller
>>>>>> code not aware of what the other part of code did?
>>>>>>
>>>>>> I raise this concern because FindBugs flags issues if the caller
>>>>>> doesn't deal with the result of the java.io.File and
>>>>>> java.util.concurrent.Lock APIs. For many that don't use FindBugs,
>>>>>> they will have a lot of bugs.
>>>>>>
>>>>>> Nathan Reynolds
>>>>>> <http://psr.us.oracle.com/__**wiki/index.php/User:Nathan___**Reynolds<http://psr.us.oracle.com/__wiki/index.php/User:Nathan___Reynolds>
>>>>>> <http://psr.us.oracle.com/**wiki/index.php/User:Nathan_**Reynolds<http://psr.us.oracle.com/wiki/index.php/User:Nathan_Reynolds>
>>>>>> >>
>>>>>> |
>>>>>>
>>>>>> On 4/21/2012 1:33 PM, Joe Bowbeer wrote:
>>>>>>
>>>>>> On Sat, Apr 21, 2012 at 12:08 PM, Aleksey Shipilev wrote:
>>>>>>
>>>>>> Yes. SettableFuture-like class is something missing from
>>>>>> concurrent classes I'm redoing over and over again in most of the
>>>>>> projects. Implementing it directly on top of AQS might provide
>>>>>> some benefits comparing to extending from FutureTask? Oh wait, it
>>>>>> smells like another API enhancement proposal? :)
>>>>>>
>>>>>>
>>>>>> Alex,
>>>>>>
>>>>>> Bill Pugh once suggested a separate concurrency abstraction: an
>>>>>> externally settable FutureValue<V>, which supports the following
>>>>>> methods:
>>>>>>
>>>>>> V get() - Waits if necessary for the value to be set, and then
>>>>>> returns the value. boolean isDone() - Returns true if the value is
>>>>>> set boolean set(V v) - Sets the value if it was not already set.
>>>>>> Returns true if the value was set by this call, false if it was
>>>>>> set by another call.
>>>>>>
>>>>>> Having set(v) return a boolean seems like a good idea. What do you
>>>>>> think?
>>>>>>
>>>>>> To this I would also add the other Future methods such as
>>>>>> cancel(), plus setException(). In other words, I envision
>>>>>> FutureValue as a Future with two additional methods: boolean
>>>>>> set(v) and boolean setException(e).
>>>>>>
>>>>>> Are there any other Future enhancements that you think are sorely
>>>>>> needed in j.u.c.?
>>>>>>
>>>>>> --Joe
>>>>>>
>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20120423/8a497d7a/attachment.html>


More information about the Concurrency-interest mailing list