From 4f4fc63fea0589a4fa03f3859dc27860cdde75af Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 12 Jun 2013 09:30:09 -0400 Subject: [PATCH 1/2] Expand microdesc cache tests Is it possible that *every* attempt to replace the microdesc cache on windows 7 is going to fail because of our lack of FILE_SHARE_DELETE while opening the file? If so, this test will catch #2077 and let us know when it's fixed. --- src/test/test_microdesc.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c index 4bc9fa7263..a8171a325c 100644 --- a/src/test/test_microdesc.c +++ b/src/test/test_microdesc.c @@ -208,11 +208,25 @@ test_md_cache(void *data) md3 = NULL; /* it's history now! */ /* rebuild again, make sure it stays gone. */ - microdesc_cache_rebuild(mc, 1); + tt_int_op(microdesc_cache_rebuild(mc, 1), ==, 0); tt_ptr_op(md1, ==, microdesc_cache_lookup_by_digest256(mc, d1)); tt_ptr_op(md2, ==, microdesc_cache_lookup_by_digest256(mc, d2)); tt_ptr_op(NULL, ==, microdesc_cache_lookup_by_digest256(mc, d3)); + /* Re-add md3, and make sure we can rebuild the cache. */ + added = microdescs_add_to_cache(mc, test_md3_noannotation, NULL, + SAVED_NOWHERE, 0, time3, NULL); + tt_int_op(1, ==, smartlist_len(added)); + md3 = smartlist_get(added, 0); + smartlist_free(added); + added = NULL; + tt_int_op(md1->saved_location, ==, SAVED_IN_CACHE); + tt_int_op(md2->saved_location, ==, SAVED_IN_CACHE); + tt_int_op(md3->saved_location, ==, SAVED_IN_JOURNAL); + + tt_int_op(microdesc_cache_rebuild(mc, 1), ==, 0); + tt_int_op(md3->saved_location, ==, SAVED_IN_CACHE); + done: if (options) tor_free(options->DataDirectory); From 884a0e269c382f9e927d8c8b1ef4ef9d2d48379d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 12 Jun 2013 09:53:46 -0400 Subject: [PATCH 2/2] Use the FILE_SHARE_DELETE flag for CreateFile on a mapping A comment by rransom on #8795 taken together with a comment by doorss recorded on #2077 suggest that *every* attempt to replace the md cache will fail on Vista/Win7 if we don't have the FILE_SHARE_DELETE flag passed to CreateFile, and if we try to replace the file ourselves before unmapping it. I'm adding the FILE_SHARE_DELETE, since that's this simplest fix. Broken indexers (the favored #2077 hypothesis) could still cause trouble here, but at least this patch should make us stop stepping on our own feet. Likely fix for #2077 and its numerous duplicates. Bugfix on 0.2.2.6-alpha, which first had a microdescriptor cache that would get replaced before remapping it. --- changes/bug2077_share_delete | 7 +++++++ src/common/compat.c | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 changes/bug2077_share_delete diff --git a/changes/bug2077_share_delete b/changes/bug2077_share_delete new file mode 100644 index 0000000000..71e9160538 --- /dev/null +++ b/changes/bug2077_share_delete @@ -0,0 +1,7 @@ + o Major bugfixes (windows): + - Open files to be mapped with FILE_SHARE_DELETE so that we can + replace them before closing the mapping. This is a likely cause of + warnings and crashes when replacing the microdescriptor cache + file. Diagnosed based on comments by "doorss" and by Robert + Ransom. Possible fix for bug 2077; bugfix on 0.2.2.6-alpha. + diff --git a/src/common/compat.c b/src/common/compat.c index 59e3898deb..7f7405f6fd 100644 --- a/src/common/compat.c +++ b/src/common/compat.c @@ -238,7 +238,7 @@ tor_mmap_file(const char *filename) strlcpy(tfilename,filename,MAX_PATH); #endif file_handle = CreateFile(tfilename, - GENERIC_READ, FILE_SHARE_READ, + GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL,