Hi, add a new ticket.
https://dev.openwrt.org/newticket
Regards
On Sat, Dec 5, 2009 at 6:22 PM, Aleksandar Radovanovic <
biblbr...@sezampro.rs> wrote:
> 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
>
--
Fábio Damião Barbosa Ricci
Escola Politécnica da Universidade de São Paulo - Engenharia Elétrica -
Ênfase Sistemas Eletrônicos
Laboratório de Sistemas Integráveis Tecnológico (LSITec) - Núcleo de
Tecnologia Sem-Fio (NTSF)
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
