Re: [Vesta-devel] Duplicate FV names -> cache assertion failure

[Kenneth C. S. <ke...@xo...>]

> 1. The evaluator shouldn't be sending duplicate names in the FV list
> when it makes an AddEntry call to the cache server.  (Unfortunately, I
> haven't yet isolated the cause of the duplicates.)

Yuan had a hunch that the problem could be that the two entries with
the matching paths could have different pathFPs.  This turned out to
be right.  (The evaluator compares the fingerprint of collected
dependency paths rather than the complete paths to save time.)

The two matching dependency paths which were causing the AddEntry to
fail were "sbin".  The two different pathFPs they had were:

16231c35db383add aa54bba5ad8afe64
96c07c9c1b33e120 71435de9427ec96a

From a little investigation, I discovered that those are the
fingerprints of the following paths (respectively):

components/kernel-headers/root/root/usr/sbin
components/kernel-headers/root/root/sbin

The both have the same final arc, so I had a hunch that early
components of the path were being removed but that the pathFP was
being left with its previous value.

There are several places in the dependency collection code in Val.C
where dependency paths are destructively modified in this fashion.
Normally these should be copies of another dependency path.  However,
I found two places where DepPath objects used in this way were
initialized with the copy constructor (which is the default
implementation).  The both looked like this:

    DepPath key1(ptr->key);

The path contained by key1 was really a pointer to the same path
acting as a key in a SharedTable.  This was causing the keys in the
table to be modified during processing, when really a copy of those
paths should have been used for this purpose.

On the assumption that this was unintended (as I couldn't imagine why
it would be), I changed these initializations to use the
DepPath::DeepCopy method.  After doing this, the duplicate paths in
the AddEntry call disappeared, as did a number of other apparently
bogus secondary dependencies.  (To be honest, I'm a little surprised
this hasn't caused noticable problems before now.)

To check for any other similar problems I made an instrumented
evaluator with checks for the correctness of the pathFPs before adding
a cache entry and at some other points.  I ran several Vesta release
builds and some larger builds here at Intel without finding any
incorrect pathFPs.  At this point I'm fairly convinced this was the
sole source of the problem.

To be on the safe side, I also changed evaluatorFunctionVersion to
force the new evaluator to miss on function cache entries created by
the old evaluator.  (I don't believe this bug affects the secondary
dependencies of _run_tool calls, so I didn't change
evaluatorRunToolVersion.)

> 2. The evaluator needs to pay attention to the return value of its
> AddEntry call.  It can't simply ignore the result and assume that a
> cache entry was created.

I also fixed this, which was fairly simple.  An AddEntry failure will
now result in the evaluation failing.

All these changes are in eval/13.

--Ken