Opened 7 years ago

Last modified 2 years ago

#7480 new defect

cached_resolve_t expiration is overengineered

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-relay, dns, refactoring, intro easy
Cc: Actual Points:
Parent ID: Points: 2
Reviewer: Sponsor:

Description

There are two misfeatures in the way we expire pending and cached DNS requests—maybe three.

  1. We schedule expiration with a priority queue. That's probably over-engineered. Instead, we could just walk the hash table periodically and remove expired entries. If we choose an appropriate interval, we shouldn't lose too much CPU to walking the hash table.
  1. We can't remove or re-order entries in the priority queue before they expire. That means that when a pending cached_resolve_t gets its answers/errors and becomes done, we don't currently remove it: instead we make a copy of it to represent the done cached_resolve_t, put that in the priority queue too, and wait for the pending one to expire on its own. How silly.
  1. Pending DNS requests probably don't need to be scheduled for expiration at all: we already have timeout logic in evdns.c to ensure that a DNS request that doesn't complete will get treated as an error. I don't think we need a separate means to expire them.

Child Tickets

Change History (14)

comment:1 Changed 4 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.2.7.x-final

Worth looking at during 0.2.7 triage IMO

comment:2 Changed 4 years ago by nickm

Status: newassigned

comment:3 Changed 4 years ago by nickm

Keywords: 027-triaged-1-out added

Marking triaged-out items from first round of 0.2.7 triage.

comment:4 Changed 4 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.???

Make all non-needs_review, non-needs_revision, 027-triaged-1-out items belong to 0.2.???

comment:5 Changed 4 years ago by nickm

Keywords: intro added

comment:6 Changed 4 years ago by nickm

Points: small/medium
Severity: Normal

comment:7 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:8 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:9 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:10 Changed 2 years ago by nickm

Keywords: 027-triaged-in added

comment:11 Changed 2 years ago by nickm

Keywords: 027-triaged-in removed

comment:12 Changed 2 years ago by nickm

Keywords: 027-triaged-1-out removed

comment:13 Changed 2 years ago by nickm

Status: assignednew

Change the status of all assigned/accepted Tor tickets with owner="" to "new".

comment:14 Changed 2 years ago by nickm

Keywords: easy added
Points: small/medium2
Note: See TracTickets for help on using tickets.