Avoid invoking overridable methods from constructors
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.