Opened 10 months ago
Closed 9 months ago
#25994 closed defect (fixed)
test_channel: keep time constant when running channel/outbound_cell
Reported by: | nickm | Owned by: | nickm |
---|---|---|---|
Priority: | Medium | Milestone: | Tor: 0.3.4.x-final |
Component: | Core Tor/Tor | Version: | |
Severity: | Normal | Keywords: | tor-ci, tor-tests-coverage, tor-tests-unit |
Cc: | Actual Points: | ||
Parent ID: | #25908 | Points: | |
Reviewer: | mikeperry | Sponsor: | Sponsor3-can |
Description
With fairly low probability, the outbound_cell test could get run with cached_gettimeofday on a different 10-second boundary than the current value of gettimeofday, resulting in a call to scale_active_circuits().
Now that we no longer use cached_gettimeofday, the probability of calling scale_active_circuits is even lower here. If the test takes about 200 microseconds (as it does on my desktop), we should only expect it to straddle the 10-second boundary with P < 200 usec / 10s == 1 / 50000.
Still, it's a good idea to fix this kind of thing.
This change will _lower_ coverage from the maximum by making it so this test does not call scale_active_circuits. However, this test does not actually validate any of the behavior of scale_active_circuits, so this change is appropriate.
Child Tickets
Change History (6)
comment:1 Changed 10 months ago by
Status: | assigned → needs_review |
---|
comment:2 Changed 10 months ago by
Parent ID: | #25993 → #25908 |
---|
comment:3 Changed 10 months ago by
Sponsor: | → Sponsor3-can |
---|
comment:4 Changed 10 months ago by
Reviewer: | → mikeperry |
---|
comment:5 Changed 9 months ago by
Status: | needs_review → needs_information |
---|
This looks simple enough to me. One question though: why not roll time forward enough for scale_active_circuits() to get called, so that we at least verify that it does not interfere with cell delivery on the channel? Unless that doesn't make sense, and there is no possible way it could.
Otherwise, merge ready.
comment:6 Changed 9 months ago by
Resolution: | → fixed |
---|---|
Status: | needs_information → closed |
Per discussion on IRC, I'd rather not actually trigger ewma scaling code here, since it was only ever triggered by accident, and this test doesn't actually do anything to check it for correctness.
Otherwise, merge ready.
Merging this to master.
https://github.com/torproject/tor/pull/73 is a pull request here; the branch is
bug25994
.