[krbdev.mit.edu #2685] profile library iterator bug

[Ken Raeburn via RT]

Problem analysis:

The iterator and the file data both contain serial numbers.

If the code notices that the file has been updated, the data is
re-read, and the serial number is incremented.  When the profile data
is "unshared" in order to make modifications, the current
implementation simply stores a null pointer and forces a re-reading of
the file data, including the serial number bump.

When an iterator is created, its serial number is copied from the file
data.  The iterator also includes a count of how many items it has
read.  If, while using the iterator, the code notices that the file
data serial number has been updated, the current pointers into the
file data are discarded, new pointers are fetched, and the code skips
over the saved number of entries with the corresponding set of labels.

So, in the simple test case I described:
 - The iterator copies the original file data serial number.
 - The first entry is returned, and the iterator counter indicates one
   entry has already been returned.
 - The file data is "unshared" and modified, with a serial number
   update.  The first entry has been deleted, and thus the new first
   entry is the former second entry.
 - The iterator code notices the serial number change, and looks up
   the new KDC list, skipping over the new first entry (which is now
   the original second entry), to point to the new second entry
   (formerly the third entry).
 - The remaining entries, except for the now-first entry, are returned
   by the iterator and deleted without problem.

So the iterator effectively never sees that first entry, and thus it
doesn't get deleted.

To fix this, perhaps the iterator should not use a simple index, but
some uid (unique per profile data tree) associated with the node.  The
"unsharing" process can duplicate the old tree, including uids, so the
iterator code can find its place more correctly.

I suspect there may also be potential problems with the iterator
hanging on to pointers to nodes that have been deleted from the tree,
storage freed, etc.  (If not for the current test case, then try one
with multiple iterators, or otherwise deleting all entries while one
iterator is in the middle of the list.)  So the deleted nodes should
probably just get a flag set, instead of being deleted immediately.
Or, if we want to be more aggressive about freeing storage as early as
possible, a ref count of iterators currently pointed to the node.

This would still leave open the problem of re-synchronizing the
iterator after re-reading the contents of a file updated externally.

Ken

P.S.  The Tcl wrapper program has been checked in in the util/profile
directory, but is not built by default, and does not get installed.
It's essentially tclsh with a few profile_* commands added.