[tor-bugs] #24587 [Core Tor/Tor]: Reset bootstrapping state on shutdown

Tor Bug Tracker & Wiki blackhole at torproject.org
Wed Jan 31 11:53:39 UTC 2018


#24587: Reset bootstrapping state on shutdown
-------------------------------------------------+-------------------------
 Reporter:  nickm                                |          Owner:  nickm
     Type:  defect                               |         Status:
                                                 |  merge_ready
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.4.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-mobile, s8-api, review-group-31  |  Actual Points:
Parent ID:  #23847                               |         Points:
 Reviewer:                                       |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by asn):

 * status:  needs_review => merge_ready


Comment:

 Hey, did some review here as part of r-g-31.

 Commit `6033538` looks very good to me.

 Commit `3809036` also looks OK, but not very happy with it. I feel like
 resetting all the global statics on `tor_free_all()` makes sense but it's
 all a very brittle logic. The moment someone adds a new global static in
 that file and doesn't know about this convention of wiping at
 `tor_free_all()`, it will introduce a bug IIUC. Furthermore, the fact that
 some of those vars get reset to 0 and others to 1 is kinda arbitrary (and
 you need to look at their definitions to see if it's correct).

 I wonder what we could do about `3809036` to make it better. Perhaps we
 should de-global those variables, put them in a struct, and initialize
 them in a function, then call that function from some entry-point and
 `tor_free_all()`? That seems like a better approach but probably not so
 trivial. Maybe subject for a future ticket on this area?

 I'll mark this as merge_ready because I think it's correct, but we should
 think about the above concerns.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24587#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online


More information about the tor-bugs mailing list