[metrics-bugs] #33205 [Metrics/Library]: Move all parsing code out of constructors

Tor Bug Tracker & Wiki blackhole at torproject.org
Mon Feb 10 08:41:05 UTC 2020


#33205: Move all parsing code out of constructors
---------------------------------+----------------------
     Reporter:  karsten          |      Owner:  karsten
         Type:  defect           |     Status:  assigned
     Priority:  Medium           |  Milestone:
    Component:  Metrics/Library  |    Version:
     Severity:  Normal           |   Keywords:
Actual Points:                   |  Parent ID:
       Points:  2                |   Reviewer:  irl
      Sponsor:                   |
---------------------------------+----------------------
 Last week I worked on parsing `"bandwidth-file-headers"` and `"bandwidth-
 file-digest"` lines from votes, and I spent way too much time on figuring
 out a mysterious bug where parsed contents would be overwritten with
 `null`.

 Turns out that the root cause was that
 `RelayNetworkStatusVoteImpl#RelayNetworkStatusVoteImpl` invoked
 `NetworkStatusImpl#NetworkStatusImpl` which indirectly invoked its own
 abstract `parseHeaders` which was overridden by
 `RelayNetworkStatusVoteImpl#parseHeaders` which stored parsed contents in
 its fields which were later initialized when constructing
 `RelayNetworkStatusVoteImpl`. In short: don't invoke overridable methods
 from constructors.

 I worked on a patch that avoids this issue in all metrics-lib classes. All
 parsing code is now contained in separate methods that need to be invoked
 explicitly and are not invoked as part of construction anymore. It's a
 larger change than I had hoped, but it's important to fix this properly. I
 tested the patch in a local metrics-test run. Attaching the branch once I
 have a ticket number.

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


More information about the metrics-bugs mailing list