Anyone?
On 11/18/2009 08:36 PM, Aleksandar Radovanovic wrote:
> Fix a memory leak in hotplug2 environment handling.
> Bump hotplug2 to the latest svn revision, remove obsolete patches.
>
> Memory leak is caused by the way hotplug2 handles environment variables,
> using setenv() and unsetenv(). setenv() creates copies of the supplied
> strings, but, due to a POSIX blunder, these copies are never destroyed
> by unsetenv(), neither in glibc nor uclibc - not until the program
> terminates.
>
> Since some events are handled directly in the main process, even when
> configured with the "fork" worker, hotplug2 memory usage will keep
> growing over time. This can be observed by running "udevtrigger" and
> noting the increase in hotplug2 VmRSS after each run.
>
> This patch uses putenv() instead, which leaves storage management to
> the caller, so that we can explicitly delete stuff when it's no longer
> needed.
>
> Signed-off-by: Aleksandar Radovanovic <biblbr...@sezampro.rs>
>
> ---
>
> Makefile | 2
> patches/100-env_memleak.patch | 64 ++++++++++++++
> patches/100-recv_check.patch | 12 --
> patches/110-static_worker.patch | 174
> +++-------------------------------------
> 4 files changed, 81 insertions(+), 171 deletions(-)
>
> Index: package/hotplug2/patches/100-env_memleak.patch
> ===================================================================
> --- package/hotplug2/patches/100-env_memleak.patch (revision 0)
> +++ package/hotplug2/patches/100-env_memleak.patch (revision 0)
> @@ -0,0 +1,64 @@
> +diff -Naur a/action.c b/action.c
> +--- a/action.c 2009-11-18 13:15:21.000000000 +0000
> ++++ b/action.c 2009-11-18 13:11:19.000000000 +0000
> +@@ -31,6 +31,30 @@
> + }
> +
> + /**
> ++ * Creates a "key=value" string from the given key and value
> ++ *
> ++ * @1 Key
> ++ * @2 Value
> ++ *
> ++ * Returns: Newly allocated string in "key=value" form
> ++ *
> ++ */
> ++static char* alloc_env(const char *key, const char *value) {
> ++ size_t keylen, vallen;
> ++ char *combined;
> ++
> ++ keylen = strlen(key);
> ++ vallen = strlen(value) + 1;
> ++
> ++ combined = xmalloc(keylen + vallen + 1);
> ++ memcpy(combined, key, keylen);
> ++ combined[keylen] = '=';
> ++ memcpy(&combined[keylen + 1], value, vallen);
> ++
> ++ return combined;
> ++}
> ++
> ++/**
> + * Choose what action should be taken according to passed settings.
> + *
> + * @1 Hotplug settings
> +@@ -41,16 +65,25 @@
> + */
> + void action_perform(struct settings_t *settings, struct uevent_t *event) {
> + int i;
> ++ char **env;
> ++
> ++ env = xmalloc(sizeof(char *) * event->env_vars_c);
> ++
> ++ for (i = 0; i < event->env_vars_c; i++) {
> ++ env[i] = alloc_env(event->env_vars[i].key,
> event->env_vars[i].value);
> ++ putenv(env[i]);
> ++ }
> +
> +- for (i = 0; i < event->env_vars_c; i++)
> +- setenv(event->env_vars[i].key, event->env_vars[i].value, 1);
> +-
> + if (settings->dumb == 0) {
> + ruleset_execute(&settings->rules, event, settings);
> + } else {
> + action_dumb(settings, event);
> + }
> +
> +- for (i = 0; i < event->env_vars_c; i++)
> ++ for (i = 0; i < event->env_vars_c; i++) {
> + unsetenv(event->env_vars[i].key);
> ++ free(env[i]);
> ++ }
> ++
> ++ free(env);
> + }
> Index: package/hotplug2/patches/100-recv_check.patch
> ===================================================================
> --- package/hotplug2/patches/100-recv_check.patch (revision 18452)
> +++ package/hotplug2/patches/100-recv_check.patch (working copy)
> @@ -1,12 +0,0 @@
> ---- a/hotplug2.c
> -+++ b/hotplug2.c
> -@@ -300,6 +300,9 @@ int main(int argc, char *argv[]) {
> - worker_ctx = settings->worker->module->init(settings);
> - while (process) {
> - size = recv(settings->netlink_socket, &buffer, sizeof(buffer),
> 0);
> -+ if (size < 0)
> -+ continue;
> -+
> - uevent = uevent_deserialize(buffer, size);
> -
> - if (uevent == NULL)
> Index: package/hotplug2/patches/110-static_worker.patch
> ===================================================================
> --- package/hotplug2/patches/110-static_worker.patch (revision 18452)
> +++ package/hotplug2/patches/110-static_worker.patch (working copy)
> @@ -1,164 +1,22 @@
> ---- a/Makefile
> -+++ b/Makefile
> -@@ -4,7 +4,24 @@ SOFTWARE=hotplug2
> - VERSION=1.0-alpha
> -
> - BINS=hotplug2 hotplug2-modwrap
> --SUBDIRS=parser rules workers
> -+SUBDIRS=parser rules
> -+
> -+hotplug2-objs := \
> -+ hotplug2.o netlink.o seqnum.o settings.o uevent.o xmemutils.o \
> -+ workers/loader.o parser/parser.o parser/buffer.o parser/token.o \
> -+ parser/token_queue.o parser/lexer.o rules/ruleset.o rules/rule.o \
> -+ rules/condition.o rules/expression.o rules/execution.o \
> -+ rules/command.o
> -+
> -+ifdef STATIC_WORKER
> -+ ifeq ($(wildcard workers/worker_$(STATIC_WORKER).c),)
> -+ $(error Worker source worker/worker_$(STATIC_WORKER).c not found)
> -+ endif
> -+ hotplug2-objs += action.o workers/worker_$(STATIC_WORKER).o
> -+else
> -+ SUBDIRS += workers
> -+endif
> -+
> - DESTDIR=
> -
> -
> -@@ -13,13 +30,16 @@ all: $(BINS)
> - install:
> - $(INSTALL_BIN) $(BINS) $(DESTDIR)/sbin/
> -
> --
> --hotplug2: hotplug2.o netlink.o seqnum.o settings.o uevent.o xmemutils.o \
> -- workers/loader.o parser/parser.o parser/buffer.o parser/token.o \
> -- parser/token_queue.o parser/lexer.o rules/ruleset.o rules/rule.o \
> -- rules/condition.o rules/expression.o rules/execution.o \
> -- rules/command.o
> -+hotplug2: $(hotplug2-objs)
> -
> - coldplug2: coldplug2.o
> -
> - include common.mak
> -+
> -+ifdef STATIC_WORKER
> -+ CFLAGS += -DSTATIC_WORKER=1
> -+else
> -+ CFLAGS += $(FPIC)
> -+ LDFLAGS += -ldl
> -+endif
> -+
> ---- a/common.mak
> -+++ b/common.mak
> -@@ -1,7 +1,10 @@
> +diff -Naur a/common.mak b/common.mak
> +--- a/common.mak 2009-11-18 13:15:21.000000000 +0000
> ++++ b/common.mak 2009-11-18 13:25:18.000000000 +0000
> +@@ -1,7 +1,7 @@
> # vim:set sw=8 nosta:
>
> --CFLAGS=-Os -Wall -g -fPIC
> + COPTS=-Os -Wall -g
> -LDFLAGS=-g -ldl
> -+COPTS=-Os -Wall -g
> +LDFLAGS=-g
> -+
> -+CFLAGS=$(COPTS)
> -+FPIC=-fPIC
>
> - INSTALL=install -c -m 644
> - INSTALL_BIN=install -c -m 755
> ---- a/workers/loader.c
> -+++ b/workers/loader.c
> -@@ -1,5 +1,23 @@
> - #include "loader.h"
> + CFLAGS=$(COPTS)
> + FPIC=-fPIC
> +diff -Naur a/Makefile b/Makefile
> +--- a/Makefile 2009-11-18 13:15:21.000000000 +0000
> ++++ b/Makefile 2009-11-18 13:25:18.000000000 +0000
> +@@ -40,5 +40,6 @@
> + CFLAGS += -DSTATIC_WORKER=1
> + else
> + CFLAGS += $(FPIC)
> ++ LDFLAGS += -ldl
> + endif
>
> -+#ifdef STATIC_WORKER
> -+
> -+extern struct worker_module_t worker_module;
> -+static struct loader_ctx_t static_ctx = {
> -+ .module = &worker_module
> -+};
> -+
> -+struct loader_ctx_t *worker_load(const char *name)
> -+{
> -+ return &static_ctx;
> -+}
> -+
> -+void worker_free(struct loader_ctx_t *ctx)
> -+{
> -+}
> -+
> -+#else
> -+
> - struct loader_ctx_t *worker_load(const char *name) {
> - struct loader_ctx_t *ctx;
> -
> -@@ -12,7 +30,7 @@ struct loader_ctx_t *worker_load(const c
> - return NULL;
> - }
> -
> -- ctx->module = dlsym(ctx->dl_handle, "module");
> -+ ctx->module = dlsym(ctx->dl_handle, "worker_module");
> - if (ctx->module == NULL) {
> - fprintf(stderr, "Loader error: %s\n", dlerror());
> - worker_free(ctx);
> -@@ -31,3 +49,5 @@ void worker_free(struct loader_ctx_t *ct
> -
> - free(ctx);
> - }
> -+
> -+#endif
> ---- a/hotplug2.c
> -+++ b/hotplug2.c
> -@@ -261,17 +261,21 @@ int main(int argc, char *argv[]) {
> - }
> -
> - /* Load the worker. */
> -+#ifndef STATIC_WORKER
> - if (settings->worker_name == NULL) {
> - fprintf(stderr, "Missing worker name.\n");
> - settings_clear(settings);
> - exit(1);
> - }
> -+#endif
> - settings->worker = worker_load(settings->worker_name);
> -+#ifndef STATIC_WORKER
> - if (settings->worker == NULL) {
> - fprintf(stderr, "Unable to load worker: %s\n",
> settings->worker_name);
> - settings_clear(settings);
> - exit(1);
> - }
> -+#endif
> -
> - /* Prepare a netlink connection to the kernel. */
> - settings->netlink_socket = netlink_init();
> ---- a/workers/worker_example.c
> -+++ b/workers/worker_example.c
> -@@ -62,7 +62,7 @@ static int worker_example_process(void *
> - return 0;
> - }
> -
> --struct worker_module_t module = {
> -+struct worker_module_t worker_module = {
> - "Hotplug2 example module",
> - worker_example_init,
> - worker_example_deinit,
> ---- a/workers/worker_fork.c
> -+++ b/workers/worker_fork.c
> -@@ -443,7 +443,7 @@ static int worker_fork_process(void *in_
> - return 0;
> - }
> -
> --struct worker_module_t module = {
> -+struct worker_module_t worker_module = {
> - "Hotplug2 forking module",
> - worker_fork_init,
> - worker_fork_deinit,
> ---- a/workers/worker_single.c
> -+++ b/workers/worker_single.c
> -@@ -18,7 +18,7 @@ static int worker_single_process(void *s
> - return 0;
> - }
> -
> --struct worker_module_t module = {
> -+struct worker_module_t worker_module = {
> - "Hotplug2 single process module",
> - worker_single_init,
> - worker_single_deinit,
> Index: package/hotplug2/Makefile
> ===================================================================
> --- package/hotplug2/Makefile (revision 18452)
> +++ package/hotplug2/Makefile (working copy)
> @@ -8,7 +8,7 @@
> include $(TOPDIR)/rules.mk
>
> PKG_NAME:=hotplug2
> -PKG_REV:=199
> +PKG_REV:=201
> PKG_VERSION:=$(PKG_REV)
> PKG_RELEASE:=1
>
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>
>
>
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
