[concurrency-interest] Attempt at yet another mostly-concurrent result cache

Joe Bowbeer joe.bowbeer at gmail.com
Tue Jan 9 09:28:29 EST 2007


I think the same hole still exists, only better camouflaged.

On 1/9/07, Holger Hoffstätte <holger at wizards.de> wrote:
>
> Peter Kovacs wrote:
> > Taking BUG1 in your first example (full method listing): what about an
> > additional check on batch readiness before adding the message:
> >
> > ...
> > synchronized (batch)
> > {
> >    if (batch.isReady())
> >    {
> >        return null;
> >    }
> >
> >    batch.add(msg);
> >
> >    if (batch.isReady())
> >    {
> > ...
>
> Then finished batches are never removed, which they should. If more
> messages with that batch key arrive, they are supposed to replace the
> previous batch; it's not a final result.
> The whole problem revolves around the fact that I want to prevent batches
> that contain one message too many at evaluation time.
>
> Hm..that got me thinking: since there's no problem that cannot be solved
> with another indirection - instead of syncing directly on the batch
> reference I could add a per-batch wrapper (like AtomicRef) and sync on
> that:
>
>     AtomicReference wrapper = cache.get(batchKey);
>     if (wrapper == null)
>     {
>         wrapper = new AtomicReference(new Batch());
>         wrapper prev = cache.putIfAbsent(batchKey, wrapper);
>         if (prev != null) wrapper = prev; // flip method-local refs
>     }
>
>     synchronized (wrapper)
>     {
>         Batch batch = wrapper.get();
>
>         // the only way for wrapper to exist but to be empty is
>         // a parallel batch completion, so use a new batch
>         // but retain our stale wrapper if possible
>         if (batch == null)
>         {
>             batch = new Batch();
>             wrapper.set(batch);
>             prev = cache.putIfAbsent(batchKey, wrapper);
>             // someone might have overtaken us again
>             if (prev != null)
>             {
>                 wrapper = prev;
>                 batch = wrapper.get();
>             }
>         }


*HERE*

        // since we might have had a stale wrapper
>         // AND possibly had to take care of a cache race
>         // (flipping the reference) we need to sync again
>         // on the new wrapper
>         synchronized (wrapper)
>         {
>             batch.add(msg);
>             if (batch.isReady())
>             {
>                 wrapper.set(null);
>                 cache.remove(batchKey);
>                 return batch.toResult();
>             }
>         }
>     }
>
> Kind of like using a WeakReference, only that I'm not checking for GC. I'm
> still not too keen on the nested sync block though I don't see how this
> could deadlock (acquire order is always A-A or A-B). But then again I
> probably messed up somewhere else..
>
> Holger
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: /pipermail/attachments/20070109/dd1e25e1/attachment.html 


More information about the Concurrency-interest mailing list