[tor-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 tor-bugs
mailing list