[krbdev.mit.edu #8431] profile_flush_to_file() can corrupt shared tree state

[Greg Hudson via RT]

 A profile data object can have two flags: DIRTY and SHARED.  DIRTY means 
there are unflushed changes in memory relative to the contents on disk, due to 
a mutating API call like profile_update_relation().  SHARED means the data 
object is part of g_shared_trees.  These flags are exclusive; mutation 
operations on a SHARED object will create a new data object with the DIRTY 
flag.

The normal flush profile_flush() operation (which is also implicit in 
profile_release()) checks the DIRTY flag and does nothing if it is not set.  
There is no bug to report here.

profile_flush_to_file() writes the contents of the profile to a specified 
file, which is presumably different from the one the profile was loaded from.    
This calls profile_flush_file_data_to_file() which calls write_data_to_file().   
The DIRTY flag is not checked, because there is no presumption that the target 
file ever contained the contents of the data object.

At the end of write_data_to_file(), data->flags is set to 0.  I assume the 
intent is to clear the DIRTY flag for a profile_flush() call, but the flag 
change is at the wrong layer.  For profile_flush_to_file(), this is harmful in 
two cases:

* For a DIRTY data object, the data object is set to non-DIRTY even though the 
profile's backing file was probably not updated.

* For a SHARED data object, the data object remains part of g_shared_trees but 
does not have the SHARED flag set.  This becomes very bad later on.  When the 
data object is dereferenced and deleted, profile_free_file_data() will free 
the data object but will not remove it from g_shared_trees, leaving a dangling 
reference.

To reproduce the second problem:

1. Open a profile.
2. Without making any changes, call profile_flush_to_file().
3. Release the profile (leaves a dangling reference in g_shared_trees).
4. Open a profile (dereferences the dangling g_shared_trees reference and 
fails an assertion).

Note that test1() in prof_test1 invokes profile_flush_to_file and then 
profile_release.  Fixing both sides bug will change the behavior of 
profile_release (the updated relations will be written to test2.ini), so we 
might need to change it to profile_abandon.