BUG/MEDIUM: servers: Use a refcount for port_range and free it properly

port_range was never freed. That used not to be a problem, but now that
we can dynamically add and remove servers, it becomes one, as that leads
to a memory leak each time a server with a "source" directive is destroyed.
However, just adding a free() is not enough. We have to add a refcount,
because the server is not the only one with a reference to it. We may
also have one in fdinfo, so that we know which port to release when we
finally close the fd.
So add a refcount, and make sure to call port_range_release() when a
server is destroyed.

This should be backported up to 3.0.

diff --git a/include/haproxy/port_range-t.h b/include/haproxy/port_range-t.h
index eea11320b1c289fba4b44bac8f4963693c2de32d..6aa0030d63ddf2a72d4614759a9f8f4238f17c65 100644 (file)
--- a/include/haproxy/port_range-t.h
+++ b/include/haproxy/port_range-t.h
@@ -27,6 +27,7 @@
 
 struct port_range {
        int size, get, put_h, put_t;    /* range size, and get/put positions */
+       int refcount;
        uint16_t ports[VAR_ARRAY];      /* array of <size> ports, in host byte order */
 };
 

diff --git a/include/haproxy/port_range.h b/include/haproxy/port_range.h
index 9e4379aff43422f584b1300578dd7f4cf9dd5cac..768b711c61338bf75f2be7cba9b71ad292ddd4e1 100644 (file)
--- a/include/haproxy/port_range.h
+++ b/include/haproxy/port_range.h
@@ -28,6 +28,17 @@
 
 #define GET_NEXT_OFF(range, off) ((off) == (range)->size - 1 ? 0 : (off) + 1)
 
+static inline void port_range_release(struct port_range *range)
+{
+       if (range) {
+               int refcount = _HA_ATOMIC_SUB_FETCH(&range->refcount, 1);
+
+               BUG_ON(refcount < 0);
+               if (refcount == 0)
+                       free(range);
+       }
+}
+
 /* return an available port from range <range>, or zero if none is left */
 static inline int port_range_alloc_port(struct port_range *range)
 {
@@ -44,6 +55,7 @@ static inline int port_range_alloc_port(struct port_range *range)
                        return 0;
                ret = range->ports[get];
        } while (!(_HA_ATOMIC_CAS(&range->get, &get, GET_NEXT_OFF(range, get))));
+       _HA_ATOMIC_INC(&range->refcount);
        return ret;
 }
 
@@ -77,6 +89,7 @@ static inline void port_range_release_port(struct port_range *range, int port)
         * can use that port.
         */
        _HA_ATOMIC_STORE(&range->put_t, GET_NEXT_OFF(range, put));
+       port_range_release(range);
 }
 
 /* return a new initialized port range of N ports. The ports are not
@@ -90,6 +103,7 @@ static inline struct port_range *port_range_alloc_range(int n)
        if (!ret)
                return NULL;
        ret->size = n + 1;
+       ret->refcount = 1;
        /* Start at the first free element */
        ret->put_h = ret->put_t = n;
        return ret;

diff --git a/src/server.c b/src/server.c
index eef885b9583e1bc3fb624bb89ffd2afba87c9585..ae5b73db84b6d7bd0eb3d1a88ac05f7b1fffcae5 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -3245,6 +3245,7 @@ void srv_free_params(struct server *srv)
                ha_free(&srv_tlv->fmt_string);
                ha_free(&srv_tlv);
        }
+       port_range_release(srv->conn_src.sport_range);
 }
 
 /* Deallocate a server <srv> and its member. <srv> must be allocated. For