Skip to content

Commit

Permalink
Fix profile_flush_to_file() state corruption
Browse files Browse the repository at this point in the history
In write_data_to_file(), do not clear the profile data object's flags.
If the call to this function resulted from profile_flush_to_file(), we
do not want to clear the DIRTY flag, and we especially do not want to
clear the SHARED flag for a data object which is part of
g_shared_trees.  Instead, clear the DIRTY flag in
profile_flush_file_data().

Add a test case to prof_test1 to exercise the bug in unfixed code.
Also modify test1 to abandon the altered profile after flushing it to
a file, to preserve the external behavior of the script before this
fix.

(cherry picked from commit 32a0599)

ticket: 8431
version_fixed: 1.14.3
  • Loading branch information
greghudson authored and tlyu committed Jul 6, 2016
1 parent dcc8b95 commit ab1aeed
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/util/profile/prof_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,6 @@ static errcode_t write_data_to_file(prf_data_t data, const char *outfile,
}
}

data->flags = 0;
retval = 0;

errout:
Expand Down Expand Up @@ -509,6 +508,7 @@ errcode_t profile_flush_file_data(prf_data_t data)
}

retval = write_data_to_file(data, data->filespec, 0);
data->flags &= ~PROFILE_FILE_DIRTY;
k5_mutex_unlock(&data->lock);
return retval;
}
Expand Down
22 changes: 21 additions & 1 deletion src/util/profile/prof_test1
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ proc test1 {} {
#profile_iterator_free $iter
catch {file delete $wd/test3.ini}
profile_flush_to_file $p $wd/test3.ini
profile_release $p
profile_abandon $p

if $verbose { puts "Reloading new profile" }
set p [profile_init_path $wd/test3.ini]
Expand Down Expand Up @@ -315,6 +315,25 @@ proc test8 {} {
puts "OK: test8: relation order in the presence of deletions"
}

proc test9 {} {
global wd verbose

# Regression test for #8431: profile_flush_to_file erroneously
# cleared the DIRTY and SHARED flags from the data object, which
# could lead to a dangling reference in g_shared_trees on release.
set p [profile_init_path $wd/test2.ini]
catch {file delete $wd/test3.ini}
profile_flush_to_file $p $wd/test3.ini
profile_release $p

# If a dangling reference was created in g_shared_trees, the next
# profile open will trigger an assertion failure.
set p [profile_init_path $wd/test2.ini]
profile_release $p

puts "OK: test9: profile_flush_to_file with no changes"
}

test1
test2
test3
Expand All @@ -323,5 +342,6 @@ test5
test6
test7
test8
test9

exit 0

0 comments on commit ab1aeed

Please sign in to comment.