Merge branch 'bug11156_issue2_squashed'

4 files changed, 46 insertions, 30 deletions

diff --git a/changes/bug11156 b/changes/bug11156
new file mode 100644
index 0000000000..bb20ed1e70
--- /dev/null
+++ b/changes/bug11156
@@ -0,0 +1,5 @@
+  o Minor bugfixes (clients):
+    - Fix a bug where a client-side Tor with pluggable transports
+      would take 60 seconds to bootstrap if a config re-read was
+      triggered at just the right timing during bootstrap. Refixes bug
+      11156; bugfix on 0.2.5.3-alpha.
\ No newline at end of file
diff --git a/src/or/config.c b/src/or/config.c
index a5407dd4ea..c3ddb5c311 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -1445,6 +1445,12 @@ options_act(const or_options_t *old_options)
   sweep_transport_list();
   sweep_proxy_list();
 
+  /* Start the PT proxy configuration. By doing this configuration
+     here, we also figure out which proxies need to be restarted and
+     which not. */
+  if (pt_proxies_configuration_pending() && !net_is_disabled())
+    pt_configure_remaining_proxies();
+
   /* Bail out at this point if we're not going to be a client or server:
    * we want to not fork, and to log stuff to stderr. */
   if (!running_tor)
diff --git a/src/or/transports.c b/src/or/transports.c
index 7e496fe219..dc30754162 100644
--- a/src/or/transports.c
+++ b/src/or/transports.c
@@ -51,35 +51,37 @@
  * logic, because of race conditions that can cause dangling
  * pointers. ]
  *
- * <b>In even more detail, this is what happens when a SIGHUP
- * occurs:</b>
+ * <b>In even more detail, this is what happens when a config read
+ * (like a SIGHUP or a SETCONF) occurs:</b>
  *
  * We immediately destroy all unconfigured proxies (We shouldn't have
- * unconfigured proxies in the first place, except when SIGHUP rings
- * immediately after tor is launched.).
+ * unconfigured proxies in the first place, except when the config
+ * read happens immediately after tor is launched.).
  *
  * We mark all managed proxies and transports to signify that they
  * must be removed if they don't contribute by the new torrc
  * (we mark using the <b>marked_for_removal</b> element).
  * We also mark all managed proxies to signify that they might need to
  * be restarted so that they end up supporting all the transports the
- * new torrc wants them to support (using the <b>got_hup</b> element).
+ * new torrc wants them to support
+ * (we mark using the <b>was_around_before_config_read</b> element).
  * We also clear their <b>transports_to_launch</b> list so that we can
  * put there the transports we need to launch according to the new
  * torrc.
  *
  * We then start parsing torrc again.
  *
- * Everytime we encounter a transport line using a known pre-SIGHUP
- * managed proxy, we cleanse that proxy from the removal mark.
- * We also mark it as unconfigured so that on the next scheduled
- * events tick, we investigate whether we need to restart the proxy
- * so that it also spawns the new transports.
- * If the post-SIGHUP <b>transports_to_launch</b> list is identical to
- * the pre-SIGHUP one, it means that no changes were introduced to
- * this proxy during the SIGHUP and no restart has to take place.
+ * Everytime we encounter a transport line using a managed proxy that
+ * was around before the config read, we cleanse that proxy from the
+ * removal mark.  We also toggle the <b>check_if_restarts_needed</b>
+ * flag, so that on the next <b>pt_configure_remaining_proxies</b>
+ * tick, we investigate whether we need to restart the proxy so that
+ * it also spawns the new transports.  If the post-config-read
+ * <b>transports_to_launch</b> list is identical to the pre-config-read
+ * one, it means that no changes were introduced to this proxy during
+ * the config read and no restart has to take place.
  *
- * During the post-SIGHUP torrc parsing, we unmark all transports
+ * During the post-config-read torrc parsing, we unmark all transports
  * spawned by managed proxies that we find in our torrc.
  * We do that so that if we don't need to restart a managed proxy, we
  * can continue using its old transports normally.
@@ -534,8 +536,7 @@ launch_managed_proxy(managed_proxy_t *mp)
 }
 
 /** Check if any of the managed proxies we are currently trying to
- *  configure have anything new to say. This is called from
- *  run_scheduled_events(). */
+ *  configure has anything new to say. */
 void
 pt_configure_remaining_proxies(void)
 {
@@ -555,11 +556,12 @@ pt_configure_remaining_proxies(void)
     tor_assert(mp->conf_state != PT_PROTO_BROKEN &&
                mp->conf_state != PT_PROTO_FAILED_LAUNCH);
 
-    if (mp->got_hup) {
-      mp->got_hup = 0;
+    if (mp->was_around_before_config_read) {
+      /* This proxy is marked by a config read. Check whether we need
+         to restart it. */
+
+      mp->was_around_before_config_read = 0;
 
-      /* This proxy is marked by a SIGHUP. Check whether we need to
-         restart it. */
       if (proxy_needs_restart(mp)) {
         log_info(LD_GENERAL, "Preparing managed proxy '%s' for restart.",
                  mp->argv[0]);
@@ -1364,19 +1366,20 @@ pt_kickstart_proxy(const smartlist_t *transport_list,
     managed_proxy_create(transport_list, proxy_argv, is_server);
 
   } else { /* known proxy. add its transport to its transport list */
-    if (mp->got_hup) {
-      /* If the managed proxy we found is marked by a SIGHUP, it means
-         that it's not useless and should be kept. If it's marked for
-         removal, unmark it and increase the unconfigured proxies so
-         that we try to restart it if we need to. Afterwards, check if
-         a transport_t for 'transport' used to exist before the SIGHUP
-         and make sure it doesn't get deleted because we might reuse
-         it. */
+    if (mp->was_around_before_config_read) {
+      /* If this managed proxy was around even before we read the
+         config this time, it means that it was already enabled before
+         and is not useless and should be kept. If it's marked for
+         removal, unmark it and make sure that we check whether it
+         needs to be restarted. */
       if (mp->marked_for_removal) {
         mp->marked_for_removal = 0;
         check_if_restarts_needed = 1;
       }
 
+      /* For each new transport, check if the managed proxy used to
+         support it before the SIGHUP. If that was the case, make sure
+         it doesn't get removed because we might reuse it. */
       SMARTLIST_FOREACH_BEGIN(transport_list, const char *, transport) {
         old_transport = transport_get_by_name(transport);
         if (old_transport)
@@ -1425,8 +1428,10 @@ pt_prepare_proxy_list_for_config_read(void)
 
     tor_assert(mp->conf_state == PT_PROTO_COMPLETED);
 
+    /* Mark all proxies for removal, and also note that they have been
+       here before the config read. */
     mp->marked_for_removal = 1;
-    mp->got_hup = 1;
+    mp->was_around_before_config_read = 1;
     SMARTLIST_FOREACH(mp->transports_to_launch, char *, t, tor_free(t));
     smartlist_clear(mp->transports_to_launch);
   } SMARTLIST_FOREACH_END(mp);
diff --git a/src/or/transports.h b/src/or/transports.h
index 7b524f2073..1365ead006 100644
--- a/src/or/transports.h
+++ b/src/or/transports.h
@@ -97,7 +97,7 @@ typedef struct {
    * this flag to signify that this proxy might need to be restarted
    * so that it can listen for other transports according to the new
    * torrc. */
-  unsigned int got_hup : 1;
+  unsigned int was_around_before_config_read : 1;
 
   /* transports to-be-launched by this proxy */
   smartlist_t *transports_to_launch;