September 2017 - tor-commits - lists.torproject.org

[stem/master] Fix test_router_status_entry for python3
by atagar＠torproject.org 04 Sep '17

04 Sep '17

commit 982c36e23931b4f54f6a5f2a2c41244c4269de95 Author: Damian Johnson <atagar(a)torproject.org> Date: Mon Sep 4 13:05:18 2017 -0700 Fix test_router_status_entry for python3 Ick, that took me a while to track down. On python3 unit tests sometimes pass, and sometimes failed with... ====================================================================== FAIL: test_router_status_entry ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/atagar/Desktop/stem/test/unit/descriptor/server_descriptor.py", line 296, in test_router_status_entry self.assertEqual('4F0069BF91C04581B7C3CA9272E2D3228D4EA571', desc.digest) AssertionError: '4F0069BF91C04581B7C3CA9272E2D3228D4EA571' != 'A863EFE8395C41C880782B89B850D20EDD242BDA' - 4F0069BF91C04581B7C3CA9272E2D3228D4EA571 + A863EFE8395C41C880782B89B850D20EDD242BDA This non-deterministic behavior was because the server descriptor's attributes were made from a dictionary, and hence had no defined ordering. With python 2.x they happened to always keep the same order, but with python3 it varied from run to run. Using OrderedDicts in this call path to ensure the order is consistent. --- stem/descriptor/__init__.py | 2 +- test/unit/descriptor/server_descriptor.py | 24 +++++++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/stem/descriptor/__init__.py b/stem/descriptor/__init__.py index 66dc573b..5c6e5f3d 100644 --- a/stem/descriptor/__init__.py +++ b/stem/descriptor/__init__.py @@ -392,7 +392,7 @@ def _descriptor_content(attr = None, exclude = (), header_template = (), footer_ """ header_content, footer_content = [], [] - attr = {} if attr is None else dict(attr) # shallow copy since we're destructive + attr = {} if attr is None else OrderedDict(attr) # shallow copy since we're destructive for content, template in ((header_content, header_template), (footer_content, footer_template)): diff --git a/test/unit/descriptor/server_descriptor.py b/test/unit/descriptor/server_descriptor.py index b31ad688..7bf0b2cf 100644 --- a/test/unit/descriptor/server_descriptor.py +++ b/test/unit/descriptor/server_descriptor.py @@ -30,6 +30,12 @@ from test.unit.descriptor import ( ) try: + # Added in 2.7 + from collections import OrderedDict +except ImportError: + from stem.util.ordereddict import OrderedDict + +try: # added in python 3.3 from unittest.mock import Mock, patch except ImportError: @@ -270,14 +276,14 @@ Qlx9HNCqCY877ztFRC624ja2ql6A2hBcuoYMbkHjcQ4= exc_msg = 'Server descriptor lacks a fingerprint. This is an optional field, but required to make a router status entry.' self.assertRaisesRegexp(ValueError, exc_msg, desc_without_fingerprint.make_router_status_entry) - desc = RelayDescriptor.create({ - 'router': 'caerSidi 71.35.133.197 9001 0 0', - 'published': '2012-02-29 04:03:19', - 'fingerprint': '4F0C 867D F0EF 6816 0568 C826 838F 482C EA7C FE44', - 'or-address': ['71.35.133.197:9001', '[12ab:2e19:3bcf::02:9970]:9001'], - 'onion-key': '\n-----BEGIN RSA PUBLIC KEY-----%s-----END RSA PUBLIC KEY-----' % stem.descriptor.CRYPTO_BLOB, - 'signing-key': '\n-----BEGIN RSA PUBLIC KEY-----%s-----END RSA PUBLIC KEY-----' % stem.descriptor.CRYPTO_BLOB, - }).make_router_status_entry() + desc = RelayDescriptor.create(OrderedDict(( + ('router', 'caerSidi 71.35.133.197 9001 0 0'), + ('published', '2012-02-29 04:03:19'), + ('fingerprint', '4F0C 867D F0EF 6816 0568 C826 838F 482C EA7C FE44'), + ('or-address', ['71.35.133.197:9001', '[12ab:2e19:3bcf::02:9970]:9001']), + ('onion-key', '\n-----BEGIN RSA PUBLIC KEY-----%s-----END RSA PUBLIC KEY-----' % stem.descriptor.CRYPTO_BLOB), + ('signing-key', '\n-----BEGIN RSA PUBLIC KEY-----%s-----END RSA PUBLIC KEY-----' % stem.descriptor.CRYPTO_BLOB), + ))).make_router_status_entry() self.assertEqual(stem.descriptor.router_status_entry.RouterStatusEntryV3, type(desc)) self.assertEqual('caerSidi', desc.nickname) @@ -293,7 +299,7 @@ Qlx9HNCqCY877ztFRC624ja2ql6A2hBcuoYMbkHjcQ4= self.assertEqual([('71.35.133.197', 9001, False), ('12ab:2e19:3bcf::02:9970', 9001, True)], desc.or_addresses) self.assertEqual(None, desc.identifier_type) self.assertEqual(None, desc.identifier) - self.assertEqual('4F0069BF91C04581B7C3CA9272E2D3228D4EA571', desc.digest) + self.assertEqual('A863EFE8395C41C880782B89B850D20EDD242BDA', desc.digest) self.assertEqual(153600, desc.bandwidth) self.assertEqual(None, desc.measured) self.assertEqual(False, desc.is_unmeasured)

1 0

[stem/master] Check for signing support in cryptography module
by atagar＠torproject.org 04 Sep '17

04 Sep '17

commit 33d41646b6de6a5a0327a95c8ef2c4c6b47e0b52 Author: Damian Johnson <atagar(a)torproject.org> Date: Mon Sep 4 12:11:02 2017 -0700 Check for signing support in cryptography module Python 3.5.2 seems to have cryptography 1.2.3. This is highly out of date and causes unit tests to fail with... ====================================================================== ERROR: test_descriptor_signing ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/atagar/Desktop/stem/test/require.py", line 58, in wrapped return func(self, *args, **kwargs) File "/home/atagar/Desktop/stem/test/unit/descriptor/extrainfo_descriptor.py", line 139, in test_descriptor_signing RelayExtraInfoDescriptor.create(sign = True) File "/home/atagar/Desktop/stem/stem/descriptor/extrainfo_descriptor.py", line 986, in create return cls(cls.content(attr, exclude, sign, signing_key), validate = validate) File "/home/atagar/Desktop/stem/stem/descriptor/extrainfo_descriptor.py", line 978, in content return _append_router_signature(content, signing_key.private) File "/home/atagar/Desktop/stem/stem/descriptor/__init__.py", line 1040, in _append_router_signature signature = base64.b64encode(private_key.sign(content, padding.PKCS1v15(), hashes.SHA1())) AttributeError: '_RSAPrivateKey' object has no attribute 'sign' ---------------------------------------------------------------------- Current version of cryptography is 2.0.3, and the sign method was added in 1.4... https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa/#cryptog… --- stem/prereq.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/stem/prereq.py b/stem/prereq.py index e3d0051d..f09870cf 100644 --- a/stem/prereq.py +++ b/stem/prereq.py @@ -98,8 +98,13 @@ def is_crypto_available(): try: from cryptography.utils import int_from_bytes, int_to_bytes from cryptography.hazmat.backends import default_backend - from cryptography.hazmat.primitives.serialization import load_der_public_key + from cryptography.hazmat.primitives.asymmetric import rsa from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes + from cryptography.hazmat.primitives.serialization import load_der_public_key + + if not hasattr(rsa.RSAPrivateKey, 'sign'): + raise ImportError() + return True except ImportError: log.log_once('stem.prereq.is_crypto_available', log.INFO, CRYPTO_UNAVAILABLE)

1 0

[tor/release-0.3.1] On windows, don't force-unlink active conscache objects.
by nickm＠torproject.org 04 Sep '17

04 Sep '17

commit 5e97b34daa27f6e0ca1c233e31d00ab8620284f0 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 29 13:02:02 2017 -0400 On windows, don't force-unlink active conscache objects. Part of a fix for bug 22752: We can't unlink these because Windows doesn't allow you to unlink an in-use file. --- src/or/conscache.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/or/conscache.c b/src/or/conscache.c index 5ffa129bb..c30d1998c 100644 --- a/src/or/conscache.c +++ b/src/or/conscache.c @@ -9,6 +9,11 @@ #define CCE_MAGIC 0x17162253 +#ifdef _WIN32 +/* On Windows, unlink won't work if there's an active mmap. */ +#define MUST_UNMAP_TO_UNLINK +#endif + /** * A consensus_cache_entry_t is a reference-counted handle to an * item in a consensus_cache_t. It can be mmapped into RAM, or not, @@ -406,23 +411,36 @@ int consensus_cache_get_n_filenames_available(consensus_cache_t *cache) { tor_assert(cache); - int max = storage_dir_get_max_files(cache->dir); + int max = cache->max_entries; int used = smartlist_len(storage_dir_list(cache->dir)); +#ifdef MUST_UNMAP_TO_UNLINK + if (used > max) + return 0; +#else tor_assert_nonfatal(max >= used); +#endif return max - used; } /** * Delete every element of cache has been marked with - * consensus_cache_entry_mark_for_removal. If force is false, - * retain those entries which are not in use except by the cache. + * consensus_cache_entry_mark_for_removal. If force is false, + * retain those entries which are in use by something other than the cache. */ void consensus_cache_delete_pending(consensus_cache_t *cache, int force) { SMARTLIST_FOREACH_BEGIN(cache->entries, consensus_cache_entry_t *, ent) { tor_assert_nonfatal(ent->in_cache == cache); - if (! force) { + int force_ent = force; +#ifdef MUST_UNMAP_TO_UNLINK + /* We cannot delete anything with an active mmap on win32, so no + * force-deletion. */ + if (ent->map) { + force_ent = 0; + } +#endif + if (! force_ent) { if (ent->refcnt > 1 || BUG(ent->in_cache == NULL)) { /* Somebody is using this entry right now */ continue;

1 0

[tor/release-0.3.1] On windows, allow many entries in conscache directories
by nickm＠torproject.org 04 Sep '17

04 Sep '17

commit da159c45e2447c729601a1393e93de908ba27c16 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 29 13:03:36 2017 -0400 On windows, allow many entries in conscache directories Since we can't be sure that we can unlink enough files on windows here, let's let the number of permitted entries grow huge if it really must. We do this by letting the storagedir hold lots of entries, but still trying to keep the number of entries under the configured limit. We also have to tell consdiffmgr not to freak out if it can't actually remove enough entries. Part of a fix for bug 22752 --- src/or/conscache.c | 27 +++++++++++++++++++++++++++ src/or/conscache.h | 1 + src/or/consdiffmgr.c | 10 +++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/or/conscache.c b/src/or/conscache.c index c30d1998c..350a05b0f 100644 --- a/src/or/conscache.c +++ b/src/or/conscache.c @@ -54,6 +54,11 @@ struct consensus_cache_t { storage_dir_t *dir; /** List of all the entries in the directory. */ smartlist_t *entries; + + /** The maximum number of entries that we'd like to allow in this cache. + * This is the same as the storagedir limit when MUST_UNMAP_TO_UNLINK is + * not defined. */ + unsigned max_entries; }; static void consensus_cache_clear(consensus_cache_t *cache); @@ -71,6 +76,10 @@ consensus_cache_open(const char *subdir, int max_entries) { consensus_cache_t *cache = tor_malloc_zero(sizeof(consensus_cache_t)); char *directory = get_datadir_fname(subdir); + cache->max_entries = max_entries; +#ifdef MUST_UNMAP_TO_UNLINK + max_entries = 1000000; +#endif cache->dir = storage_dir_new(directory, max_entries); tor_free(directory); if (!cache->dir) { @@ -82,6 +91,19 @@ consensus_cache_open(const char *subdir, int max_entries) return cache; } +/** Return true if it's okay to put more entries in this cache than + * its official file limit. */ +int +consensus_cache_may_overallocate(consensus_cache_t *cache) +{ + (void) cache; +#ifdef MUST_UNMAP_TO_UNLINK + return 1; +#else + return 0; +#endif +} + /** * Tell the sandbox (if any) configured by cfg to allow the * operations that cache will need. @@ -90,6 +112,11 @@ int consensus_cache_register_with_sandbox(consensus_cache_t *cache, struct sandbox_cfg_elem **cfg) { +#ifdef MUST_UNMAP_TO_UNLINK + /* Our sandbox doesn't support huge limits like we use here. + */ + tor_assert_nonfatal_unreached(); +#endif return storage_dir_register_with_sandbox(cache->dir, cfg); } diff --git a/src/or/conscache.h b/src/or/conscache.h index aef54201f..a0d74c4e0 100644 --- a/src/or/conscache.h +++ b/src/or/conscache.h @@ -14,6 +14,7 @@ HANDLE_DECL(consensus_cache_entry, consensus_cache_entry_t, ) consensus_cache_t *consensus_cache_open(const char *subdir, int max_entries); void consensus_cache_free(consensus_cache_t *cache); struct sandbox_cfg_elem; +int consensus_cache_may_overallocate(consensus_cache_t *cache); int consensus_cache_register_with_sandbox(consensus_cache_t *cache, struct sandbox_cfg_elem **cfg); void consensus_cache_unmap_lazy(consensus_cache_t *cache, time_t cutoff); diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c index 928fc26f5..831d5d45c 100644 --- a/src/or/consdiffmgr.c +++ b/src/or/consdiffmgr.c @@ -1136,7 +1136,7 @@ consdiffmgr_ensure_space_for_files(int n) return 0; } // Let's get more assertive: clean out unused stuff, and force-remove - // the files. + // the files that we can. consdiffmgr_cleanup(); consensus_cache_delete_pending(cache, 1); const int n_to_remove = n - consensus_cache_get_n_filenames_available(cache); @@ -1159,6 +1159,14 @@ consdiffmgr_ensure_space_for_files(int n) smartlist_free(objects); consensus_cache_delete_pending(cache, 1); + + if (consensus_cache_may_overallocate(cache)) { + /* If we're allowed to throw extra files into the cache, let's do so + * rather getting upset. + */ + return 0; + } + if (BUG(n_marked < n_to_remove)) return -1; else

1 0

[tor/release-0.3.1] Merge branch 'bug22752_031_simple' into maint-0.3.1
by nickm＠torproject.org 04 Sep '17

04 Sep '17

commit 4d97efd4d1cdc39e42fd0aeaeeee9accfeb15785 Merge: a7aeb6f58 948be49ce Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Sep 4 15:48:01 2017 -0400 Merge branch 'bug22752_031_simple' into maint-0.3.1 changes/bug22752_simple | 6 ++++ src/or/conscache.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++--- src/or/conscache.h | 1 + src/or/consdiffmgr.c | 10 +++++- 4 files changed, 95 insertions(+), 6 deletions(-)

1 0

[tor/release-0.3.1] 22752: Improve comments to explain why we're doing this fix.
by nickm＠torproject.org 04 Sep '17

04 Sep '17

commit 948be49ce06f744ca456d20f48bfb6d2f5cfb7d2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Sep 4 11:54:49 2017 -0400 22752: Improve comments to explain why we're doing this fix. Based on questions and comments from dgoulet, I've tried to fill in the reasoning about why these functions work in the way that they do, so that it will be easier for future programmers to understand why this code exists and works the way it does. --- src/or/conscache.c | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/or/conscache.c b/src/or/conscache.c index 350a05b0f..9e13ce8e4 100644 --- a/src/or/conscache.c +++ b/src/or/conscache.c @@ -10,7 +10,10 @@ #define CCE_MAGIC 0x17162253 #ifdef _WIN32 -/* On Windows, unlink won't work if there's an active mmap. */ +/* On Windows, unlink won't work on a file if the file is actively mmap()ed. + * That forces us to be less aggressive about unlinking files, and causes other + * changes throughout our logic. + */ #define MUST_UNMAP_TO_UNLINK #endif @@ -74,13 +77,26 @@ static void consensus_cache_entry_unmap(consensus_cache_entry_t *ent); consensus_cache_t * consensus_cache_open(const char *subdir, int max_entries) { + int storagedir_max_entries; consensus_cache_t *cache = tor_malloc_zero(sizeof(consensus_cache_t)); char *directory = get_datadir_fname(subdir); cache->max_entries = max_entries; + #ifdef MUST_UNMAP_TO_UNLINK - max_entries = 1000000; + /* If we can't unlink the files that we're still using, then we need to + * tell the storagedir backend to allow far more files than this consensus + * cache actually wants, so that it can hold files which, from this cache's + * perspective, have become useless. + */ +#define VERY_LARGE_STORAGEDIR_LIMIT (1000*1000) + storagedir_max_entries = VERY_LARGE_STORAGEDIR_LIMIT; +#else + /* Otherwise, we can just tell the storagedir to use the same limits + * as this cache. */ + storagedir_max_entries = max_entries; #endif - cache->dir = storage_dir_new(directory, max_entries); + + cache->dir = storage_dir_new(directory, storagedir_max_entries); tor_free(directory); if (!cache->dir) { tor_free(cache); @@ -92,7 +108,12 @@ consensus_cache_open(const char *subdir, int max_entries) } /** Return true if it's okay to put more entries in this cache than - * its official file limit. */ + * its official file limit. + * + * (We need this method on Windows, where we can't unlink files that are still + * in use, and therefore might need to temporarily exceed the file limit until + * the no-longer-wanted files are deletable.) + */ int consensus_cache_may_overallocate(consensus_cache_t *cache) { @@ -113,7 +134,15 @@ consensus_cache_register_with_sandbox(consensus_cache_t *cache, struct sandbox_cfg_elem **cfg) { #ifdef MUST_UNMAP_TO_UNLINK - /* Our sandbox doesn't support huge limits like we use here. + /* Our Linux sandbox doesn't support huge file lists like the one that would + * be generated by using VERY_LARGE_STORAGEDIR_LIMIT above in + * consensus_cache_open(). Since the Linux sandbox is the only one we have + * right now, we just assert that we never reach this point when we've had + * to use VERY_LARGE_STORAGEDIR_LIMIT. + * + * If at some point in the future we have a different sandbox mechanism that + * can handle huge file lists, we can remove this assertion or make it + * conditional. */ tor_assert_nonfatal_unreached(); #endif

1 0

[tor/release-0.3.1] Merge branch 'maint-0.3.1' into release-0.3.1
by nickm＠torproject.org 04 Sep '17

04 Sep '17

commit c515fbc7327ce1189c0b511265b2bd6b452003e3 Merge: 3a5cf9aa2 4d97efd4d Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Sep 4 15:48:12 2017 -0400 Merge branch 'maint-0.3.1' into release-0.3.1 changes/bug22752_simple | 6 ++++ src/or/conscache.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++--- src/or/conscache.h | 1 + src/or/consdiffmgr.c | 10 +++++- 4 files changed, 95 insertions(+), 6 deletions(-)

1 0

[tor/release-0.3.1] Changes file for bug22752 (simple version)
by nickm＠torproject.org 04 Sep '17

04 Sep '17

commit a58a41c15a53edda1e560221bd17e46c8df6a661 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 29 13:09:39 2017 -0400 Changes file for bug22752 (simple version) --- changes/bug22752_simple | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changes/bug22752_simple b/changes/bug22752_simple new file mode 100644 index 000000000..7e6035705 --- /dev/null +++ b/changes/bug22752_simple @@ -0,0 +1,6 @@ + o Major bugfixes (windows, directory cache): + - On windows, do not try to delete cached consensus documents and + diffs, until they unmapped from memory. Allow the diff storage + directory to grow larger in order to handle files that might + need to stay around longer. Fixes bug 22752; bugfix on + 0.3.1.1-alpha.

1 0

[tor/maint-0.3.1] On windows, allow many entries in conscache directories
by nickm＠torproject.org 04 Sep '17

04 Sep '17

commit da159c45e2447c729601a1393e93de908ba27c16 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 29 13:03:36 2017 -0400 On windows, allow many entries in conscache directories Since we can't be sure that we can unlink enough files on windows here, let's let the number of permitted entries grow huge if it really must. We do this by letting the storagedir hold lots of entries, but still trying to keep the number of entries under the configured limit. We also have to tell consdiffmgr not to freak out if it can't actually remove enough entries. Part of a fix for bug 22752 --- src/or/conscache.c | 27 +++++++++++++++++++++++++++ src/or/conscache.h | 1 + src/or/consdiffmgr.c | 10 +++++++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/or/conscache.c b/src/or/conscache.c index c30d1998c..350a05b0f 100644 --- a/src/or/conscache.c +++ b/src/or/conscache.c @@ -54,6 +54,11 @@ struct consensus_cache_t { storage_dir_t *dir; /** List of all the entries in the directory. */ smartlist_t *entries; + + /** The maximum number of entries that we'd like to allow in this cache. + * This is the same as the storagedir limit when MUST_UNMAP_TO_UNLINK is + * not defined. */ + unsigned max_entries; }; static void consensus_cache_clear(consensus_cache_t *cache); @@ -71,6 +76,10 @@ consensus_cache_open(const char *subdir, int max_entries) { consensus_cache_t *cache = tor_malloc_zero(sizeof(consensus_cache_t)); char *directory = get_datadir_fname(subdir); + cache->max_entries = max_entries; +#ifdef MUST_UNMAP_TO_UNLINK + max_entries = 1000000; +#endif cache->dir = storage_dir_new(directory, max_entries); tor_free(directory); if (!cache->dir) { @@ -82,6 +91,19 @@ consensus_cache_open(const char *subdir, int max_entries) return cache; } +/** Return true if it's okay to put more entries in this cache than + * its official file limit. */ +int +consensus_cache_may_overallocate(consensus_cache_t *cache) +{ + (void) cache; +#ifdef MUST_UNMAP_TO_UNLINK + return 1; +#else + return 0; +#endif +} + /** * Tell the sandbox (if any) configured by cfg to allow the * operations that cache will need. @@ -90,6 +112,11 @@ int consensus_cache_register_with_sandbox(consensus_cache_t *cache, struct sandbox_cfg_elem **cfg) { +#ifdef MUST_UNMAP_TO_UNLINK + /* Our sandbox doesn't support huge limits like we use here. + */ + tor_assert_nonfatal_unreached(); +#endif return storage_dir_register_with_sandbox(cache->dir, cfg); } diff --git a/src/or/conscache.h b/src/or/conscache.h index aef54201f..a0d74c4e0 100644 --- a/src/or/conscache.h +++ b/src/or/conscache.h @@ -14,6 +14,7 @@ HANDLE_DECL(consensus_cache_entry, consensus_cache_entry_t, ) consensus_cache_t *consensus_cache_open(const char *subdir, int max_entries); void consensus_cache_free(consensus_cache_t *cache); struct sandbox_cfg_elem; +int consensus_cache_may_overallocate(consensus_cache_t *cache); int consensus_cache_register_with_sandbox(consensus_cache_t *cache, struct sandbox_cfg_elem **cfg); void consensus_cache_unmap_lazy(consensus_cache_t *cache, time_t cutoff); diff --git a/src/or/consdiffmgr.c b/src/or/consdiffmgr.c index 928fc26f5..831d5d45c 100644 --- a/src/or/consdiffmgr.c +++ b/src/or/consdiffmgr.c @@ -1136,7 +1136,7 @@ consdiffmgr_ensure_space_for_files(int n) return 0; } // Let's get more assertive: clean out unused stuff, and force-remove - // the files. + // the files that we can. consdiffmgr_cleanup(); consensus_cache_delete_pending(cache, 1); const int n_to_remove = n - consensus_cache_get_n_filenames_available(cache); @@ -1159,6 +1159,14 @@ consdiffmgr_ensure_space_for_files(int n) smartlist_free(objects); consensus_cache_delete_pending(cache, 1); + + if (consensus_cache_may_overallocate(cache)) { + /* If we're allowed to throw extra files into the cache, let's do so + * rather getting upset. + */ + return 0; + } + if (BUG(n_marked < n_to_remove)) return -1; else

1 0

[tor/maint-0.3.1] 22752: Improve comments to explain why we're doing this fix.
by nickm＠torproject.org 04 Sep '17

04 Sep '17

commit 948be49ce06f744ca456d20f48bfb6d2f5cfb7d2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Sep 4 11:54:49 2017 -0400 22752: Improve comments to explain why we're doing this fix. Based on questions and comments from dgoulet, I've tried to fill in the reasoning about why these functions work in the way that they do, so that it will be easier for future programmers to understand why this code exists and works the way it does. --- src/or/conscache.c | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/or/conscache.c b/src/or/conscache.c index 350a05b0f..9e13ce8e4 100644 --- a/src/or/conscache.c +++ b/src/or/conscache.c @@ -10,7 +10,10 @@ #define CCE_MAGIC 0x17162253 #ifdef _WIN32 -/* On Windows, unlink won't work if there's an active mmap. */ +/* On Windows, unlink won't work on a file if the file is actively mmap()ed. + * That forces us to be less aggressive about unlinking files, and causes other + * changes throughout our logic. + */ #define MUST_UNMAP_TO_UNLINK #endif @@ -74,13 +77,26 @@ static void consensus_cache_entry_unmap(consensus_cache_entry_t *ent); consensus_cache_t * consensus_cache_open(const char *subdir, int max_entries) { + int storagedir_max_entries; consensus_cache_t *cache = tor_malloc_zero(sizeof(consensus_cache_t)); char *directory = get_datadir_fname(subdir); cache->max_entries = max_entries; + #ifdef MUST_UNMAP_TO_UNLINK - max_entries = 1000000; + /* If we can't unlink the files that we're still using, then we need to + * tell the storagedir backend to allow far more files than this consensus + * cache actually wants, so that it can hold files which, from this cache's + * perspective, have become useless. + */ +#define VERY_LARGE_STORAGEDIR_LIMIT (1000*1000) + storagedir_max_entries = VERY_LARGE_STORAGEDIR_LIMIT; +#else + /* Otherwise, we can just tell the storagedir to use the same limits + * as this cache. */ + storagedir_max_entries = max_entries; #endif - cache->dir = storage_dir_new(directory, max_entries); + + cache->dir = storage_dir_new(directory, storagedir_max_entries); tor_free(directory); if (!cache->dir) { tor_free(cache); @@ -92,7 +108,12 @@ consensus_cache_open(const char *subdir, int max_entries) } /** Return true if it's okay to put more entries in this cache than - * its official file limit. */ + * its official file limit. + * + * (We need this method on Windows, where we can't unlink files that are still + * in use, and therefore might need to temporarily exceed the file limit until + * the no-longer-wanted files are deletable.) + */ int consensus_cache_may_overallocate(consensus_cache_t *cache) { @@ -113,7 +134,15 @@ consensus_cache_register_with_sandbox(consensus_cache_t *cache, struct sandbox_cfg_elem **cfg) { #ifdef MUST_UNMAP_TO_UNLINK - /* Our sandbox doesn't support huge limits like we use here. + /* Our Linux sandbox doesn't support huge file lists like the one that would + * be generated by using VERY_LARGE_STORAGEDIR_LIMIT above in + * consensus_cache_open(). Since the Linux sandbox is the only one we have + * right now, we just assert that we never reach this point when we've had + * to use VERY_LARGE_STORAGEDIR_LIMIT. + * + * If at some point in the future we have a different sandbox mechanism that + * can handle huge file lists, we can remove this assertion or make it + * conditional. */ tor_assert_nonfatal_unreached(); #endif

1 0