[tor-bugs] #22752 [Core Tor/Tor]: Assertion failure in consensus_cache_entry_handle_get on Windows

Tor Bug Tracker & Wiki blackhole at torproject.org
Wed Aug 30 12:18:56 UTC 2017


#22752: Assertion failure in consensus_cache_entry_handle_get on Windows
--------------------------+------------------------------------
 Reporter:  ahf           |          Owner:  nickm
     Type:  defect        |         Status:  needs_revision
 Priority:  High          |      Milestone:  Tor: 0.3.1.x-final
Component:  Core Tor/Tor  |        Version:  Tor: 0.3.1.3-alpha
 Severity:  Normal        |     Resolution:
 Keywords:                |  Actual Points:
Parent ID:                |         Points:
 Reviewer:  dgoulet       |        Sponsor:  Sponsor4
--------------------------+------------------------------------
Changes (by dgoulet):

 * status:  needs_review => needs_revision
 * reviewer:   => dgoulet


Comment:

 * I see this that is a very puzzling thing for which I think I figured it
 out:

 {{{
 +  cache->max_entries = max_entries;
 +#ifdef MUST_UNMAP_TO_UNLINK
 +  max_entries = 1000000;
 +#endif
    cache->dir = storage_dir_new(directory, max_entries);
 }}}

  I've looked on how `consensus_cache_open()` is used and how big
 `max_entries` can be and seems it will always be (128 * 2) so the value in
 the case of Windows is drastically bigger (1000000). And that explains the
 changes file saying that we want a storage dir bigger than the actual
 limit of the cache.

  That code above needs a comment explaining this because I got very
 confused on this arbitrary limit of 1000000 and the limit vs the
 allocation difference.

 * In `consensus_cache_register_with_sandbox()`:

 {{{
 +#ifdef MUST_UNMAP_TO_UNLINK
 +  /* Our sandbox doesn't support huge limits like we use here.
 +   */
 +  tor_assert_nonfatal_unreached();
 +#endif
 }}}

  And also no sandbox on Windows but let us assume that we can enable that
 define on OperatingSystem9000 for instance, we should indicate here that
 the fix for the requirement to UNMAP is to have a huge file limit thus not
 compatible with our sandbox because the link between "sandbox" and "must
 unmap" is not easy to do...

 * `consensus_cache_may_overallocate()` imo should have documentation on
 *why* it does exists in the first place. If I was new to the code, I would
 look at the function which would lead me to `#define MUST_UNMAP_TO_UNLINK`
 which only has the comment "/* On Windows, unlink won't work if there's an
 active mmap. */" which would let me very confused.

  I would like somewhere a clear comment on the why of the relationship
 between "MUST UNMAP" and the fact that we have a very large max entry
 value for that as a fix.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22752#comment:19>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online


More information about the tor-bugs mailing list