Re: [BUG] resolved deltas

[Jeff King <peff@peff.net>]

On Mon, Aug 25, 2014 at 06:39:45PM +0200, René Scharfe wrote:

> Thanks, that looks good.  But while preparing the patch I noticed that
> the added test sometimes fails.  Helgrind pointed outet a race
> condition.  It is not caused by the patch to turn the asserts into
> regular ifs, however -- here's a Helgrind report for the original code
> with the new test:

Interesting. I couldn't convince Helgrind to catch such a case, but it
makes sense. We split the delta-resolving work by dividing up the base
objects. We then find any deltas that need that base object (which is
read-only). If there's only one instances of the base, then we'll be the
only thread working on those delta. But if there are two such bases,
they're going to look at the same deltas.

So we need some kind of mutual exclusion so that only one thread
proceeds with resolving the delta. The "real_type" check sort-of
functions in that way (except of course it is not actually thread safe).
So one obvious option is just a coarse-grained global lock to modify or
check real_type fields. That would probably perform badly (lots of
useless lock contention on unrelated objects), but at least it would
work.

If we accept pushing a lock into _each_ object_entry, then we would get
a lot less lock contention (i.e., none at all in the common case of no
duplicates). The cost is storing one lock per object, though. Not great,
but probably OK.

Is there some way we can make real_type itself more atomic? I.e., use it
as the mutex with the rule that we do not "claim" the object_entry as
ours to work on until we atomically set delta_obj->real_type. I think
doing a compare-and-swap looking for OBJ_REF_DELTA and replacing it with
base->real_type would be enough there (and substitute OBJ_OFS_DELTA for
the second conditional, of course).

However, I'm not sure we can portably rely on having a compare-and-swap
primitive (or that we want to go through the hassle of conditionally
using it).

-Peff