Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#3019 closed defect (fixed)

setconf clears bridge descriptor refetch timers

Reported by: arma Owned by: arma
Priority: High Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-bridge
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I was looking at #2355's hopes of refetching bridge descriptors as soon as the bridge list changes. My plan was to simply call fetch_bridge_descriptors(now) from inside options_act() once we've cleared and reparsed the bridge lines.

Then I realized that clearing and reparsing the bridge lines resets the timers. So currently Tor clients that use bridges will refetch all their bridge descriptors an average of 30 seconds after each setconf. Bug.

Better behavior might be to leave parsed bridges alone if they're the same.

Child Tickets

Change History (8)

comment:1 Changed 8 years ago by arma

Owner: set to arma
Status: newassigned

comment:2 Changed 8 years ago by arma

Priority: normalmajor

comment:3 Changed 8 years ago by nickm

Status: assignedneeds_review

stole this one from you. See my branch "bug3019".

comment:4 Changed 8 years ago by arma

Looks good to me. Thanks.

You should 'make check-spaces' it before merging.

Also, I notice the idiom

      SMARTLIST_DEL_CURRENT(bridge_list, b);
      tor_free(b);

and wonder if freeing b after removing it from the smartlist is ok. I think it is. But that said, we do it the other way in dirserv.c:

          tor_free(cp);
          SMARTLIST_DEL_CURRENT(fps_out, cp);

Is this something we should try to develop a consistent habit around?

comment:5 Changed 8 years ago by nickm

Thanks for reviewing; I'll make the check-spaces test patch and merge.

wrt the free-then-remove or remove-then-free, it doesn't matter at all for smartlists. For other types (like hashtables or a hypothetical linked list type), you really do want to remove-then-free. I would suggest a general remove-then-free habit, but not strongly enough to run around changing existing cases.

comment:6 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

comment:7 Changed 7 years ago by nickm

Keywords: tor-bridge added

comment:8 Changed 7 years ago by nickm

Component: Tor BridgeTor
Note: See TracTickets for help on using tickets.