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

David M. Lloyd david.lloyd at redhat.com
Mon Apr 23 14:33:34 EDT 2012


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>>
>>> |
>>>
>>> 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
>>
>>
>


-- 
- DML


More information about the Concurrency-interest mailing list