I'm working with a framework that requires a callback when sending a request. Each callback has to implement this interface. The methods in the callback are invoked asynchronously.
public interface ClientCallback<RESP extends Response>
{
public void onSuccessResponse(RESP resp);
public void onFailureResponse(FailureResponse failure);
public void onError(Throwable e);
}
To write integration tests with TestNG, I wanted to have a blocking callback. So I used a CountDownLatch to synchronize between threads.
Is the AtomicReference really needed here or is a raw reference okay? I know that if I use a raw reference and a raw integer (instead of CountDownLatch), the code wouldn't work because visibility is not guaranteed. But since the CountDownLatch is already synchronized, I wasn't sure whether I needed the extra synchronization from AtomicReference. Note: The Result class is immutable.
public class BlockingCallback<RESP extends Response> implements ClientCallback<RESP>
{
private final AtomicReference<Result<RESP>> _result = new AtomicReference<Result<RESP>>();
private final CountDownLatch _latch = new CountDownLatch(1);
public void onSuccessResponse(RESP resp)
{
_result.set(new Result<RESP>(resp, null, null));
_latch.countDown();
}
public void onFailureResponse(FailureResponse failure)
{
_result.set(new Result<RESP>(null, failure, null));
_latch.countDown();
}
public void onError(Throwable e)
{
_result.set(new Result<RESP>(null, null, e));
_latch.countDown();
}
public Result<RESP> getResult(final long timeout, final TimeUnit unit) throws InterruptedException, TimeoutException
{
if (!_latch.await(timeout, unit))
{
throw new TimeoutException();
}
return _result.get();
}
You don't need to use another synchronization object (AtomicRefetence) here. The point is that the variable is set before CountDownLatch is invoked in one thread and read after CountDownLatch is invoked in another thread. CountDownLatch already performs thread synchronization and invokes memory barrier so the order of writing before and reading after is guaranteed. Because of this you don't even need to use volatile for that field.
A good starting point is the javadoc (emphasis mine):
Memory consistency effects: Until the count reaches zero, actions in a thread prior to calling
countDown()
happen-before actions following a successful return from a correspondingawait()
in another thread.
Now there are two options:
onXxx
setter methods once the count is 0 (i.e. you only call one of the methods once) and you don't need any extra synchronizationIf you are in scenario 2, you need to make the variable at least volatile
(no need for an AtomicReference
in your example).
If you are in scenario 1, you need to decide how defensive you want to be:
volatile
onXxx
methods is guaranteed to be visibleFinally, in scenario 1, you may want to enforce the fact that the setters can only be called once, in which case you would probably use an AtomicReference
and its compareAndSet
method to make sure that the reference was null beforehand and throw an exception otherwise.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With