Opened 8 months ago

Closed 8 months ago

#28554 closed defect (fixed)

Fix memory leaks and missing unmocks in test_entry_guard_outdated_dirserver_exclusion

Reported by: teor Owned by: teor
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor: 0.3.0.1-alpha
Severity: Normal Keywords: bootstrap, clock-skew, s8-bootstrap, 033-backport-maybe, 034-backport-maybe, 035-backport
Cc: Actual Points: 0.1
Parent ID: #24661 Points: 0.1
Reviewer: catalyst Sponsor: Sponsor8-can

Description

test_entry_guard_outdated_dirserver_exclusion leaks memory, and is missing some unmocks

Child Tickets

Change History (13)

comment:1 Changed 8 months ago by teor

I included the fix in my #24661 branch, but we might want to backport it.

comment:2 Changed 8 months ago by teor

Actual Points: 0.1
Points: 0.1
Sponsor: Sponsor8-can
Status: assignedneeds_review

This ticket is sponsor8-can, because it fixes some unit tests we need for #24661.

See my pull request:
https://github.com/torproject/tor/pull/534

comment:3 Changed 8 months ago by teor

This branch is based on maint-0.3.3.

comment:4 Changed 8 months ago by teor

The stem test will fail due to #28571, and the appveyor tests fail due to #28574.

comment:5 Changed 8 months ago by dgoulet

Reviewer: catalyst

comment:6 Changed 8 months ago by catalyst

Status: needs_reviewneeds_information

Thanks! The pull request is against master, rather than 0.3.3, which appears to be the base branch. Would you like to adjust the pull request? Or is there some reason not to?

comment:7 Changed 8 months ago by teor

Status: needs_informationneeds_review

master is the default, and I wasn't paying attention.

Here's a pull request for 0.3.3:
https://github.com/torproject/tor/pull/550

Here's a pull request for master, because 0.3.4 and later have Windows tests:
https://github.com/torproject/tor/pull/534

I think we've fixed all the outstanding CI issues, so everything should pass.

comment:8 Changed 8 months ago by teor

Oh, hang on, the 0.3.3 pull request CI points to master for some reason. I think GitHub's CI API is broken, or it's broken on the Travis/Appveyor side.

Here's the 0.3.3 branch CI from my repository:
https://travis-ci.org/teor2345/tor/builds/458279337

comment:9 in reply to:  8 Changed 8 months ago by catalyst

Replying to teor:

Oh, hang on, the 0.3.3 pull request CI points to master for some reason. I think GitHub's CI API is broken, or it's broken on the Travis/Appveyor side.

Here's the 0.3.3 branch CI from my repository:
https://travis-ci.org/teor2345/tor/builds/458279337

It looks like your bug28554-033 and teor/bug28554 branches are identical for some reason? At least from what I fetch from GitHub.

$ git rev-parse teor/bug28554-033 teor/bug28554
ffc7b81b5dd909f0c4325e7a5b893504f76b9c77
ffc7b81b5dd909f0c4325e7a5b893504f76b9c77

comment:10 in reply to:  3 Changed 8 months ago by teor

Yes, the branches are identical:

Replying to teor:

This branch is based on maint-0.3.3.

comment:11 Changed 8 months ago by catalyst

OK weird. The Travis pages show the correct base branches, but attribute the build to the wrong pull request. Oddly enough, the links in the GitHub checks details go to the correct builds.

comment:12 Changed 8 months ago by catalyst

Status: needs_reviewmerge_ready

Patches look good to me. Thanks!

comment:13 Changed 8 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Merged to 0.3.3 and forward!

Note: See TracTickets for help on using tickets.