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

Gregg Wonderly gregg at cytetech.com
Mon Apr 23 14:22:50 EDT 2012


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>>
>> |
>>
>> 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
>>
>>
>>
>> _______________________________________________
>> Concurrency-interest mailing list
>> Concurrency-interest at cs.oswego.edu
>> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>
>



More information about the Concurrency-interest mailing list