Bug 3519

X-url: http://www.isi.edu/~johnh/
To: rdist-dev
Subject: correction to rdist memory consumption problem
(with fix)
Fcc: week, outbox
--------

On Feb. 4th I posted a message about a problem:

	I've found a serious memory consumption problem for rdist
when running
	over directories that contain many hard-linked files.  The
attached
	patch fixes this problem at the cost of ~12 more lines of
code and a
	little more dynamic memory allocation.

	Rdist allocates a fixed-sized buffer (struct linkbuf in
defs.h) for
	each file that has more than one hard link.  This buffer
includes
	pre-allocated space for 3 strings, each of length BUFSIZ.
Undef
	Redhat 5.2/linux-2.0.36 BUFSIZ is 8KB, so a lot of this
buffer goes to
	waste.

	For my mh mailbox directory (mh---even worse than news
spools at
	stressing file systems!), I have ~54k directory entires,
~35k files
	with only one link and ~8300 with 2-7 links.  With typical
rdist this
	was resulting memory usage of >90MB and much swapping on my
poor
	laptop.  With the included fix memory for this case is
<3MB.

(This problem can also be found at
http://developer.redhat.com/bugzilla/show_bug.cgi?id=1046&BUGLIST=1046.)

Unfortuantely, there was a bug in the patch that I posted
that
breaks rdist'ing files with multiple links in certain
circumstances.
(If Tdest isn't set when linkinfo is called.)

Here's the fix, relative to my prior patch:
> --- client.c-   Mon Jun  7 15:54:59 1999
> +++ client.c    Mon Jun  7 15:58:12 1999
> @@ -347,8 +347,8 @@
>         if (Tdest)
>                 lp->target = strdup(Tdest);
>         else
> -               lp->target = NULL;
> -       if (!lp->pathname || !lp->src || !(Tdest &&
lp->target))
> +               lp->target = strdup("");
> +       if (!lp->pathname || !lp->src || !lp->target)
>                 fatalerr("Cannot malloc memory in
linkinfo.");
>
>         return((struct linkbuf *) NULL);

(Basically if Tdest was null, lp->target was always null and
the
fatalerr was always triggered.)


Alternatively, here's a replacement original patch (with the
bug
fixed):

----------------------------------------
--- src/client.c-	Wed Jun 16 17:34:57 1999
+++ src/client.c	Wed Jun 16 17:35:57 1999
@@ -309,6 +309,18 @@
 	return(0);
 }

+void freelinkinfo(lp)
+	struct linkbuf *lp;
+{
+	if (lp->pathname)
+		free(lp->pathname);
+	if (lp->src)
+		free(lp->src);
+	if (lp->target)
+		free(lp->target);
+	free(lp);
+}
+
 /*
  * Save and retrieve hard link info
  */
@@ -317,6 +329,7 @@
 {
 	struct linkbuf *lp;

+	/* xxx: linear search doesn't scale with many links */
 	for (lp = ihead; lp != NULL; lp = lp->nextp)
 		if (lp->inum == statp->st_ino && lp->devnum ==
statp->st_dev) {
 			lp->count--;
@@ -329,12 +342,14 @@
 	lp->inum = statp->st_ino;
 	lp->devnum = statp->st_dev;
 	lp->count = statp->st_nlink - 1;
-	(void) strcpy(lp->pathname, target);
-	(void) strcpy(lp->src, source);
+	lp->pathname = strdup(target);
+	lp->src = strdup(source);
 	if (Tdest)
-		(void) strcpy(lp->target, Tdest);
+		lp->target = strdup(Tdest);
 	else
-		*lp->target = CNULL;
+		lp->target = strdup("");
+	if (!lp->pathname || !lp->src || !lp->target)
+		fatalerr("Cannot malloc memory in linkinfo.");

 	return((struct linkbuf *) NULL);
 }
--- src/docmd.c-	Wed Jun 16 17:35:07 1999
+++ src/docmd.c	Wed Jun 16 17:35:18 1999
@@ -586,7 +586,7 @@
 	if (!nflag) {
 		register struct linkbuf *nextl, *l;

-		for (l = ihead; l != NULL; free((char *)l), l = nextl) {
+		for (l = ihead; l != NULL; freelinkinfo(l), l = nextl) {
 			nextl = l->nextp;
 			if (contimedout || IS_ON(opts, DO_IGNLNKS) ||
 			    l->count == 0)
--- include/defs.h-	Wed Jun 16 17:35:14 1999
+++ include/defs.h	Wed Jun 16 17:35:18 1999
@@ -276,9 +276,9 @@
 	ino_t	inum;
 	dev_t	devnum;
 	int	count;
-	char	pathname[BUFSIZ];
-	char	src[BUFSIZ];
-	char	target[BUFSIZ];
+	char	*pathname;
+	char	*src;
+	char	*target;
 	struct	linkbuf *nextp;
 };

----------------------------------------

This patch can replace RedHat's rdist-6.1.5-links.patch to
fix the
problem.

To reproduce the problem, create a directory like
/home/johnh/TMP/RDIST_TEST, populate it with the commands
	cd /home/johnh/TMP/RDIST_TEST
	touch a b
	ln b bb
	cat >distfile <<END

HOSTS = ( boreas.isi.edu )

master: /home/johnh/TMP/RDIST_TEST -> ${HOSTS}
        install -w -y;
END

then exercise the bug
with
	rdist -d HOSTS=otherhost.of.yours.com -f
/home/johnh/TMP/RDIST_TEST/distfile

with the incorrect fix rdist will exit with the error
"Cannot malloc memory in linkinfo.".
The correct version will run to completion.

My apologies for sending in a buggy first fix.

   -John Heidemann