[tor-bugs] #29930 [Core Tor/Tor]: Warning: can't unlink unverified-consensus on Windows

[Tor Bug Tracker & Wiki]

#29930: Warning: can't unlink unverified-consensus on Windows
--------------------------------+------------------------------------
 Reporter:  teor                |          Owner:  ahf
     Type:  defect              |         Status:  assigned
 Priority:  Medium              |      Milestone:  Tor: 0.4.0.x-final
Component:  Core Tor/Tor        |        Version:  Tor: 0.4.0.3-alpha
 Severity:  Normal              |     Resolution:
 Keywords:  040-must, Windows?  |  Actual Points:
Parent ID:                      |         Points:  1
 Reviewer:                      |        Sponsor:
--------------------------------+------------------------------------

Comment (by ahf):

 Tried to debug this on Linux to see what could cause the file to remain
 opened at the time of the call to unlink(). On Linux the following system
 calls are made around the problematic time of execution:

 {{{
  15 rename("/home/user/torrc-test//cached-certs.tmp", "/home/user/torrc-
 test//cached-certs") = 0
  14 openat(AT_FDCWD, "/home/user/torrc-test//unverified-microdesc-
 consensus", O_RDONLY|O_CLOEXEC) = 9
  13 fstat(9, {st_mode=S_IFREG|0600, st_size=2124535, ...}) = 0
  12 mmap(NULL, 2124535, PROT_READ, MAP_PRIVATE, 9, 0) = 0x77db35a74000
  11 close(9)                                = 0
     [ ... brk calls ... ]
   0 unlink("/home/user/torrc-test//unverified-microdesc-consensus") = 0
 }}}

 On Windows the `tor_mmap_file()` function will keep the file open until we
 call `tor_munmap_file()`. This is a problem in
 `reload_consensus_from_file()` since we will do the following:

 1. mmap() the `unverified-microdesc-consensus` file.
 2. Call `networkstatus_set_current_consensus()` with the content of the
 memory mapped file
 3. `networkstatus_set_current_consensus()` will call `unlink()` on
 `unverified-microdesc-consensus`, which is still open because of (1) and
 will thus fail.
 4. We will munmap() the file again when
 `networkstatus_set_current_consensus()` returns to
 `reload_consensus_from_file()`.

 I'm not sure what a good solution would be here. We need to either get rid
 of the `mmap()` call or lift the call to `unlink()` out of the block of
 code where the file that is to be unlinked is still memory mapped.

 One option is to avoid using mmap(), but use `read_file_to_str()`. I think
 though that we moved to mmap() in this code to be more gentle with memory
 usage for iOS. Maybe we can use read_file_to_str() for Windows only, or?

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