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

Gregg Wonderly gregg at cytetech.com
Mon Apr 23 17:33:52 EDT 2012


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