NetBSD Problem Report #48444

From www@NetBSD.org  Thu Dec 12 11:17:27 2013
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 9890EA63C4
	for <gnats-bugs@gnats.NetBSD.org>; Thu, 12 Dec 2013 11:17:27 +0000 (UTC)
Message-Id: <20131212111725.9919AA642D@mollari.NetBSD.org>
Date: Thu, 12 Dec 2013 11:17:25 +0000 (UTC)
From: sdaoden@gmail.com
Reply-To: sdaoden@gmail.com
To: gnats-bugs@NetBSD.org
Subject: mail(1)/mailx(1): relaxation of memory pressure
X-Send-Pr-Version: www-1.0

>Number:         48444
>Category:       bin
>Synopsis:       mail(1)/mailx(1): relaxation of memory pressure
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Dec 12 11:20:00 +0000 2013
>Originator:     Steffen
>Release:        6.99.24
>Organization:
>Environment:
-
>Description:
    sreset() drops all the auto-reclaimed storage and thus cannot
    be used when iterating over all messages in a mailbox, because
    there may have been allocations in that storage somewhere before.

Because of this the BSD Mail codebase simply iterates over all messages that belong to a message-selection, allocating more and more memory.

    Therefore introduce a temporary additional layer on top of that
    which can be used by such code paths without resetting yet
    existent allocations.

   /* The purpose of relaxation is only that it is possible to reset the
    * casters, *not* to give back memory to the system.  We are presumably in
    * an iteration over all messages of a mailbox, and it'd be quite
    * counterproductive to give the system allocator a chance to waste time */
>How-To-Repeat:
Open a (MIME-enabled) mail(1) instance on a mailbox with many messages and enter a condition where many messages are processed in order, e.g., 'view *', or change a message status flag ('unre NUMBER') when the mailbox is editable and leave the mailbox, causing an updated version to be written.
>Fix:
diff -Napru nmail/cmd1.c nmail.relax/cmd1.c
--- nmail/cmd1.c	2013-12-09 13:16:52.000000000 +0100
+++ nmail.relax/cmd1.c	2013-12-09 19:09:03.000000000 +0100
@@ -124,13 +124,16 @@ headers(void *v)
 	flag = 0;
 	if (dot != get_message(n))
 		dot = mp;
+	srelax_on();
 	for (/*EMPTY*/; mp; mp = next_message(mp)) {
 		if (mp->m_flag & MDELETED)
 			continue;
 		if (flag++ >= size)
 			break;
 		printhead(get_msgnum(mp));
+		srelax();
 	}
+	srelax_off();
 	if (flag == 0) {
 		(void)printf("No more mail.\n");
 		return 1;
@@ -343,6 +346,7 @@ type1(int *msgvec, int doign, int mime_d
 	 * exit code will never be seen.
 	 */
 	sig_check();
+	srelax_on();
 	oldsigpipe = sig_signal(SIGPIPE, cmd1_brokpipe);
 	if (setjmp(pipestop))
 		goto close_pipe;
@@ -365,8 +369,10 @@ type1(int *msgvec, int doign, int mime_d
 		args.mip = NULL;
 #endif
 		(void)thread_recursion(mp, type1_core, &args);
+		srelax();
 	}
 close_pipe:
+	srelax_off();
 #ifdef MIME_SUPPORT
 	if (mip != NULL) {
 		struct sigaction osa;
@@ -549,6 +555,7 @@ top(void *v)
 	args.lineb = 1;
 	recursive = do_recursion();
 	msgCount = get_msgCount();
+	srelax_on();
 	for (ip = msgvec; *ip && ip - msgvec < msgCount; ip++) {
 		struct message *mp;

@@ -556,7 +563,9 @@ top(void *v)
 		dot = mp;
 		args.parent = recursive ? mp : NULL;
 		(void)thread_recursion(mp, top_core, &args);
+		srelax();
 	}
+	srelax_off();
 	return 0;
 }

diff -Napru nmail/cmd2.c nmail.relax/cmd2.c
--- nmail/cmd2.c	2013-12-09 13:16:52.000000000 +0100
+++ nmail.relax/cmd2.c	2013-12-09 19:02:25.000000000 +0100
@@ -234,6 +234,7 @@ save1(char str[], int markmsg, const cha
 		warn(NULL);
 		return 1;
 	}
+	srelax_on();
 	for (ip = msgvec; *ip && ip - msgvec < msgCount; ip++) {
 		struct save1_core_args_s args;
 		struct message *mp;
@@ -245,9 +246,12 @@ save1(char str[], int markmsg, const cha
 		if (thread_recursion(mp, save1_core, &args)) {
 			warn("%s", fn);
 			(void)Fclose(obuf);
+			srelax_off();
 			return 1;
 		}
+		srelax();
 	}
+	srelax_off();
 	(void)fflush(obuf);
 	if (ferror(obuf))
 		warn("%s", fn);
@@ -690,6 +694,7 @@ detach1(void *v, int do_unnamed)
 		msgvec[1] = 0;
 	}
 	recursive = do_recursion();
+	srelax_on();
 	for (ip = msgvec; *ip && ip - msgvec < msgCount; ip++) {
 		struct detach1_core_args_s args;
 		struct message *mp;
@@ -700,7 +705,9 @@ detach1(void *v, int do_unnamed)
 		args.igtab = do_unnamed ? detachall : ignoreall;
 		args.dstdir = dstdir;
 		(void)thread_recursion(mp, detach1_core, &args);
+		srelax();
 	}
+	srelax_off();
 	return 0;
 }

diff -Napru nmail/extern.h nmail.relax/extern.h
--- nmail/extern.h	2013-12-09 13:16:52.000000000 +0100
+++ nmail.relax/extern.h	2013-12-09 18:36:12.000000000 +0100
@@ -278,6 +278,9 @@ int	sendmail(void *);
 void *	csalloc(size_t, size_t);
 void *	salloc(size_t);
 void	sreset(void);
+void	srelax_on(void);
+void	srelax_off(void);
+void	srelax(void);
 void	spreserve(void);

 /*
diff -Napru nmail/list.c nmail.relax/list.c
--- nmail/list.c	2013-12-09 13:16:52.000000000 +0100
+++ nmail.relax/list.c	2013-12-09 19:06:33.000000000 +0100
@@ -845,6 +845,7 @@ match_string(int *markarray, char *str, 
 		return -1;

 	rval = 0;
+	srelax_on();
 	for (i = 1; i <= msgCount; i++) {
 		struct message *mp;
 		mp = get_message(i);
@@ -854,7 +855,9 @@ match_string(int *markarray, char *str, 
 		if (rval)
 			markarray[i - 1] = 1;
 		rval = 0;
+		srelax();
 	}
+	srelax_off();

 	free_cmparg(cmparg);	/* free any memory allocated by get_cmpfn() */

@@ -1325,9 +1328,13 @@ show_headers_and_exit(int flags)
 		(void)signal(SIGINT, SIG_IGN);

 	flags &= CMMASK;
+	srelax_on();
 	for (mp = get_message(1); mp; mp = next_message(mp))
-		if (flags == 0 || !ignore_message(mp->m_flag, flags))
+		if (flags == 0 || !ignore_message(mp->m_flag, flags)) {
 			printhead(get_msgnum(mp));
+			srelax();
+		}
+	srelax_off();

 	exit(0);
 	/* NOTREACHED */
diff -Napru nmail/quit.c nmail.relax/quit.c
--- nmail/quit.c	2013-12-09 13:16:52.000000000 +0100
+++ nmail.relax/quit.c	2013-12-09 18:59:56.000000000 +0100
@@ -95,15 +95,19 @@ writeback(FILE *res)
 		}
 	}
 #endif
+	srelax_on();
 	for (mp = get_message(1); mp; mp = next_message(mp))
 		if ((mp->m_flag & MPRESERVE) || (mp->m_flag & MTOUCH)==0) {
 			p++;
 			if (sendmessage(mp, obuf, NULL, NULL, NULL) < 0) {
 				warn("%s", mailname);
 				(void)Fclose(obuf);
+				srelax_off();
 				return -1;
 			}
+			srelax();
 		}
+	srelax_off();
 #ifdef APPEND
 	if (res != NULL)
 		while ((c = getc(res)) != EOF)
@@ -221,16 +225,20 @@ edstop(jmp_buf jmpbuf)
 	}
 	trunc(obuf);
 	c = 0;
+	srelax_on();
 	for (mp = get_message(1); mp; mp = next_message(mp)) {
 		if ((mp->m_flag & MDELETED) != 0)
 			continue;
 		c++;
 		if (sendmessage(mp, obuf, NULL, NULL, NULL) < 0) {
 			warn("%s", mailname);
+			srelax_off();
 			sig_release();
 			longjmp(jmpbuf, -1);
 		}
+		srelax();
 	}
+	srelax_off();
 	gotcha = (c == 0 && ibuf == NULL);
 	if (ibuf != NULL) {
 		while ((c = getc(ibuf)) != EOF)
@@ -480,17 +488,22 @@ nolock:
 		}
 		(void)fchmod(fileno(obuf), 0600);
 	}
+	srelax_on();
 	for (mp = get_message(1); mp; mp = next_message(mp))
-		if (mp->m_flag & MBOX)
+		if (mp->m_flag & MBOX) {
 			if (sendmessage(mp, obuf, saveignore, NULL, NULL) < 0) {
 				warn("%s", mbox);
 				if (!append)
 					(void)Fclose(ibuf);
 				(void)Fclose(obuf);
 				(void)Fclose(fbuf);
+				srelax_off();
 				dot_unlock(mailname);
 				return;
 			}
+			srelax();
+		}
+	srelax_off();

 	/*
 	 * Copy the user's old mbox contents back
diff -Napru nmail/strings.c nmail.relax/strings.c
--- nmail/strings.c	2013-12-09 13:16:52.000000000 +0100
+++ nmail.relax/strings.c	2013-12-09 20:08:08.000000000 +0100
@@ -38,6 +38,8 @@ __RCSID("$NetBSD: strings.c,v 1.18 2010/
 #endif
 #endif /* not lint */

+#include <assert.h>
+
 /*
  * Mail -- a mail program
  *
@@ -60,10 +62,13 @@ __RCSID("$NetBSD: strings.c,v 1.18 2010/
 #define	NSPACE	25			/* Total number of string spaces */
 static struct strings {
 	char	*s_topFree;		/* Beginning of this area */
+	char	*s_relaxFree;		/* Begin of area during relaxation */
 	char	*s_nextFree;		/* Next alloctable place here */
 	size_t	s_nleft;		/* Number of bytes left here */
 } stringdope[NSPACE];

+static int	relaxation;
+
 /*
  * Allocate size more bytes of space and return the address of the
  * first byte to the caller.  An even number of bytes are always
@@ -97,6 +102,7 @@ salloc(size_t size)
 		sp->s_topFree = malloc(STRINGSIZE << idx);
 		if (sp->s_topFree == NULL)
 			errx(EXIT_FAILURE, "No room for space %d", idx);
+		sp->s_relaxFree = NULL;
 		sp->s_nextFree = sp->s_topFree;
 		sp->s_nleft = STRINGSIZE << idx;
 	}
@@ -131,13 +137,64 @@ sreset(void)

 	if (noreset)
 		return;
+
+	relaxation = 0;
+
 	idx = 0;
-	for (sp = &stringdope[0]; sp < &stringdope[NSPACE]; sp++) {
+	for (sp = &stringdope[0]; sp < &stringdope[NSPACE]; ++idx, ++sp) {
 		if (sp->s_topFree == NULL)
 			continue;
+		sp->s_relaxFree = NULL;
 		sp->s_nextFree = sp->s_topFree;
 		sp->s_nleft = STRINGSIZE << idx;
-		idx++;
+	}
+}
+
+PUBLIC void
+srelax_on(void)
+{
+	struct strings *sp;
+	int idx;
+
+	assert(relaxation == 0);
+
+	idx = 0;
+	for (sp = &stringdope[0]; sp < &stringdope[NSPACE]; ++idx, ++sp) {
+		if (sp->s_topFree == NULL)
+			continue;
+		sp->s_relaxFree = sp->s_nextFree;
+	}
+
+	relaxation = 1;
+}
+
+PUBLIC void
+srelax_off(void)
+{
+	assert(relaxation == 1);
+
+	srelax();
+	relaxation = 0;
+}
+
+PUBLIC void
+srelax(void)
+{
+	struct strings *sp;
+	int idx;
+
+	assert(relaxation == 1);
+
+	idx = 0;
+	for (sp = &stringdope[0]; sp < &stringdope[NSPACE]; ++idx, ++sp) {
+		if (sp->s_topFree == NULL)
+			continue;
+		sp->s_nleft = STRINGSIZE << idx;
+		if ((sp->s_nextFree = sp->s_relaxFree) == NULL)
+			sp->s_nextFree = sp->s_topFree;
+		else
+			sp->s_nleft -= (size_t)(uintptr_t)(sp->s_relaxFree -
+			    sp->s_topFree);
 	}
 }

Home	PR Database Search