okaywit / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Race condition in AbstractFuture.addListener(...) #1496

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I'm using a library built on guava (Cassandra CQL  java driver from DataStax) 
which allows asynchronous execution of queries. That is, the ResultSetFuture 
allows to get(...) with a timeout and also to add a listener which is called 
when the result is available. Internally this depends on 
AbstractFuture.addListener(...)

I'm wondering whether the behaviour of AbstractFuture related to 
addListener(...) can be considered a race condition. I tend to think so: the 
documentation of ListenableFuture.addListener(...) indicates that 'The listener 
will run when the Future's computation is complete or, if the computation is 
already complete, immediately'.

However, looking at the AbstractFuture implementation, I cannot identify code 
which would result in this behavior. If set(...) is called after 
addListener(...), the listeners are run. However, if the set(...) method is 
called before addListener(...), the added listener never gets run since it is 
only added to the executionList.

I think I have witnessed this behavior in the aforementioned Cassandra CQL java 
driver. I hammer a Cassandra cluster with asynchronous queries until a maximum 
amount of queries is pending. The pending counter gets decremented when a 
listener to the ResultSetFuture (which extends AbstractFuture) is ran. However, 
many of them don't get run. Which can be explained given my 'race condition in 
AbstractFuture.addListener(...) hypothesis'.

Original issue reported on code.google.com by frens...@frensjan.nl on 5 Aug 2013 at 7:37

GoogleCodeExporter commented 9 years ago
ExecutionList.add() runs the listener if ExecutionList.execute() has already 
been called. Since execute() is called when AbstractFuture.set() is called, 
calls to AbstractFuture.addListener() that happen after set() will run the 
listener.

Original comment by cgdecker@google.com on 5 Aug 2013 at 7:52

GoogleCodeExporter commented 9 years ago
I stand corrected, thanks for the quick response!

Original comment by frens...@frensjan.nl on 5 Aug 2013 at 7:54

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:12

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08