Browse code

Fixed issue where struct env_set methods that change the value of an existing name=value pair would delay the freeing of the memory held by the previous name=value pair until the underlying client instance object is closed.

This could cause a server that handles long-term
client connections, resulting in many periodic calls
to verify_callback, to needlessly grow the env_set
memory allocation until the underlying client instance
object is closed.


git-svn-id: http://svn.openvpn.net/projects/openvpn/branches/BETA21/openvpn@1493 e7ae566f-a301-0410-adde-c780ea21d3b5

james authored on 2006/11/24 07:05:14
Showing 4 changed files
... ...
@@ -2272,10 +2272,22 @@ do_close_ifconfig_pool_persist (struct context *c)
2272 2272
 static void
2273 2273
 do_inherit_env (struct context *c, const struct env_set *src)
2274 2274
 {
2275
-  c->c2.es = env_set_create (&c->c2.gc);
2275
+  c->c2.es = env_set_create (NULL);
2276
+  c->c2.es_owned = true;
2276 2277
   env_set_inherit (c->c2.es, src);
2277 2278
 }
2278 2279
 
2280
+static void
2281
+do_env_set_destroy (struct context *c)
2282
+{
2283
+  if (c->c2.es && c->c2.es_owned)
2284
+    {
2285
+      env_set_destroy (c->c2.es);
2286
+      c->c2.es = NULL;
2287
+      c->c2.es_owned = false;
2288
+    }
2289
+}
2290
+
2279 2291
 /*
2280 2292
  * Fast I/O setup.  Fast I/O is an optimization which only works
2281 2293
  * if all of the following are true:
... ...
@@ -2798,6 +2810,9 @@ close_instance (struct context *c)
2798 2798
 	/* close --ifconfig-pool-persist obj */
2799 2799
 	do_close_ifconfig_pool_persist (c);
2800 2800
 
2801
+	/* free up environmental variable store */
2802
+	do_env_set_destroy (c);
2803
+
2801 2804
 	/* garbage collect */
2802 2805
 	gc_free (&c->c2.gc);
2803 2806
       }
... ...
@@ -2922,6 +2937,7 @@ inherit_context_top (struct context *dest,
2922 2922
   dest->c2.event_set_owned = false;
2923 2923
   dest->c2.link_socket_owned = false;
2924 2924
   dest->c2.buffers_owned = false;
2925
+  dest->c2.es_owned = false;
2925 2926
 
2926 2927
   dest->c2.event_set = NULL;
2927 2928
   if (src->options.proto == PROTO_UDPv4)
... ...
@@ -690,13 +690,13 @@ add_env_item (char *str, const bool do_alloc, struct env_item **list, struct gc_
690 690
 static bool
691 691
 env_set_del_nolock (struct env_set *es, const char *str)
692 692
 {
693
-  return remove_env_item (str, false, &es->list);
693
+  return remove_env_item (str, es->gc == NULL, &es->list);
694 694
 }
695 695
 
696 696
 static void
697 697
 env_set_add_nolock (struct env_set *es, const char *str)
698 698
 {
699
-  remove_env_item (str, false, &es->list);  
699
+  remove_env_item (str, es->gc == NULL, &es->list);  
700 700
   add_env_item ((char *)str, true, &es->list, es->gc);
701 701
 }
702 702
 
... ...
@@ -704,7 +704,6 @@ struct env_set *
704 704
 env_set_create (struct gc_arena *gc)
705 705
 {
706 706
   struct env_set *es;
707
-  ASSERT (gc);
708 707
   mutex_lock_static (L_ENV_SET);
709 708
   ALLOC_OBJ_CLEAR_GC (es, struct env_set, gc);
710 709
   es->list = NULL;
... ...
@@ -713,6 +712,25 @@ env_set_create (struct gc_arena *gc)
713 713
   return es;
714 714
 }
715 715
 
716
+void
717
+env_set_destroy (struct env_set *es)
718
+{
719
+  mutex_lock_static (L_ENV_SET);
720
+  if (es && es->gc == NULL)
721
+    {
722
+      struct env_item *e = es->list;
723
+      while (e)
724
+	{
725
+	  struct env_item *next = e->next;
726
+	  free (e->string);
727
+	  free (e);
728
+	  e = next;
729
+	}
730
+      free (es);
731
+    }
732
+  mutex_unlock_static (L_ENV_SET);
733
+}
734
+
716 735
 bool
717 736
 env_set_del (struct env_set *es, const char *str)
718 737
 {
... ...
@@ -168,6 +168,7 @@ void setenv_del (struct env_set *es, const char *name);
168 168
 /* struct env_set functions */
169 169
 
170 170
 struct env_set *env_set_create (struct gc_arena *gc);
171
+void env_set_destroy (struct env_set *es);
171 172
 bool env_set_del (struct env_set *es, const char *str);
172 173
 void env_set_add (struct env_set *es, const char *str);
173 174
 
... ...
@@ -403,6 +403,7 @@ struct context_2
403 403
 
404 404
   /* environmental variables to pass to scripts */
405 405
   struct env_set *es;
406
+  bool es_owned;
406 407
 
407 408
   /* don't wait for TUN/TAP/UDP to be ready to accept write */
408 409
   bool fast_io;