[tor-bugs] #29706 [Core Tor/Tor]: Test failure due to memory leaks in shared-random unit tests: long-term fix (was: Test failure due to memory leaks in shared-random unit tests)

[Tor Bug Tracker & Wiki]

#29706: Test failure due to memory leaks in shared-random unit tests: long-term fix
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  teor
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Very High                            |      Milestone:  Tor:
                                                 |  0.4.1.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  0.2.9.1-alpha
 Severity:  Normal                               |     Resolution:
 Keywords:  consider-backport-after-authority-   |  Actual Points:  0.5
  test, consider-backport-after-0404-alpha,      |
  tor-ci, tor-test, memory-management, 034       |
  -backport-maybe, 035-backport, 040-backport    |
Parent ID:                                       |         Points:  0.5
 Reviewer:  asn                                  |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by teor):

 * status:  needs_revision => needs_review
 * keywords:
     tor-ci, tor-test, memory-management, 034-backport-maybe, 035-backport,
     040-backport
     =>
     consider-backport-after-authority-test, consider-backport-
     after-0404-alpha, tor-ci, tor-test, memory-management, 034-backport-
     maybe, 035-backport, 040-backport


Comment:

 Replying to [comment:4 nickm]:
 > quick question here -- when we are about to call state_query_del_, do we
 need a check to make sure that the value we are about to assign is not the
 same pointer as the previous value?

 I think replacing a pointer with itself is almost always a bug, because:
 * we've confused current and previous, or
 * we forgot sr_srv_dup().

 Unless both pointers are NULL: replacing NULL with NULL is ok.

 I changed the code so it does nothing when replacing a non-NULL pointer
 with the same pointer, and logs a BUG() warning. That BUG() doesn't
 trigger on the unit tests or chutney.

 The changes are in ​https://github.com/torproject/tor/pull/776
 I re-did the merge, because I ended up with a lot of partial merges.

 We should merge it to 0.4.0 and master, then consider a backport to 0.3.4
 and 0.3.5 once arma has run it on moria for a while (and once it's been in
 at least one full alpha).

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