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

Ariel Weisberg ariel at weisberg.ws
Mon Apr 23 16:07:04 EDT 2012


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