ports/102414: [PATCH] mail/cyrus-imapd: avoid fopen limit during mbox copy

Sean Farley sean-freebsd at farley.org
Wed Aug 23 02:30:15 UTC 2006


>Number:         102414
>Category:       ports
>Synopsis:       [PATCH] mail/cyrus-imapd: avoid fopen limit during mbox copy
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-ports-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          change-request
>Submitter-Id:   current-users
>Arrival-Date:   Wed Aug 23 02:30:14 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Sean Farley
>Release:        FreeBSD 6.1-STABLE i386
>Organization:
>Environment:
System: FreeBSD thor.farley.org 6.1-STABLE FreeBSD 6.1-STABLE #0: Thu Jul 20 15:22:07 CDT 2006
>Description:
Here is a copy of the bug I reported to cyrus-bugs+ at andrew.cmu.edu:
---------------------
I am in the process of migrating from the UWash IMAP server to the Cyrus
IMAP server (v2.3.7).  While attempting a move via Pine of a large
folder (1700+ messages), the Cyrus IMAP server would generate an error:

Aug 21 22:15:13 mail imap[20414]: couldn't create stage directory:
/var/spool/imap/stage./: File exists
Aug 21 22:15:13 mail imap[20414]: IOERROR: creating message file
/var/spool/imap/stage./20414-1156216500-242: File exists

This was however not the true cause of the problem.  fopen() was failing
when hitting its hard limit of 256 open FILE's.  The problem is in
imapd.c:cmd_append() where it keeps the FILE open until it is finished
with it.  I have written a patch[1] that fclose()'s each file when it is
finished with it and fopen()'s it for parsing if it is a binary message.
---------------------

I kept the patch as one file since I hope that it will be fixed soon, and
the fix needs to change those three files.  Separation into three files
is fine with me.

Added file(s):
- files/patch-fopen

Port maintainer (ume at FreeBSD.org) is cc'd.

Generated with FreeBSD Port Tools 0.77
>How-To-Repeat:
>Fix:

--- cyrus-imapd-2.3.7.patch begins here ---
diff -ruN --exclude=CVS /usr/ports/mail/cyrus-imapd23.orig/files/patch-fopen /usr/ports/mail/cyrus-imapd23/files/patch-fopen
--- /usr/ports/mail/cyrus-imapd23.orig/files/patch-fopen	Wed Dec 31 18:00:00 1969
+++ /usr/ports/mail/cyrus-imapd23/files/patch-fopen	Tue Aug 22 21:09:06 2006
@@ -0,0 +1,114 @@
+--- imap/append.c.orig	Thu May 25 07:57:31 2006
++++ imap/append.c	Tue Aug 22 19:31:09 2006
+@@ -395,6 +395,17 @@
+ }
+ 
+ /*
++ * Return the stage filename
++ */
++
++const char *append_stagename(struct stagemsg *stagep)
++{
++    if (!stagep) return NULL;
++
++    return (stagep->fname);
++}
++
++/*
+  * staging, to allow for single-instance store.  initializes the stage
+  * with the file for the given mailboxname and returns the open file
+  * so it can double as the spool file
+--- imap/append.h.orig	Thu Jun  2 11:16:02 2005
++++ imap/append.h	Tue Aug 22 19:35:24 2006
+@@ -120,6 +120,8 @@
+ 
+ int append_stageparts(struct stagemsg *stagep);
+ 
++const char *append_stagename(struct stagemsg *stagep);
++
+ /* creates a new stage and returns stage file corresponding to mailboxname */
+ extern FILE *append_newstage(const char *mailboxname, time_t internaldate,
+ 			     int msgnum, struct stagemsg **stagep);
+--- imap/imapd.c.orig	Mon Jul  3 08:22:41 2006
++++ imap/imapd.c	Tue Aug 22 20:38:58 2006
+@@ -2959,6 +2959,9 @@
+     char *newserver;
+     int numalloc = 5;
+     struct appendstage *curstage;
++    char stagedir[MAX_MAILBOX_PATH+1];
++    char stagefile[MAX_MAILBOX_PATH+1];
++    FILE *fileStage;
+ 
+     /* See if we can append */
+     r = (*imapd_namespace.mboxname_tointernal)(&imapd_namespace, name,
+@@ -3098,8 +3101,8 @@
+ 	}
+ 
+ 	/* Stage the message */
+-	curstage->f = append_newstage(mailboxname, now, numstage, &(curstage->stage));
+-	if (!curstage->f) {
++	fileStage = append_newstage(mailboxname, now, numstage, &(curstage->stage));
++	if (!fileStage) {
+ 	    r = IMAP_IOERROR;
+ 	    goto done;
+ 	}
+@@ -3113,8 +3116,8 @@
+ 
+ 	    /* Catenate the message part(s) to stage */
+ 	    size = 0;
+-	    r = append_catenate(curstage->f, cur_name, &size,
+-				&(curstage->binary), &parseerr, &url);
++	    r = append_catenate(fileStage, cur_name, &size, &(curstage->binary),
++				&parseerr, &url);
+ 	    if (r) goto done;
+ 	}
+ 	else {
+@@ -3123,8 +3126,10 @@
+ 	    if (r) goto done;
+ 
+ 	    /* Copy message to stage */
+-	    r = message_copy_strict(imapd_in, curstage->f, size, curstage->binary);
++	    r = message_copy_strict(imapd_in, fileStage, size, curstage->binary);
+ 	}
++	fclose(fileStage);
++	fileStage = NULL;
+ 	totalsize += size;
+ 
+ 	/* if we see a SP, we're trying to append more than one message */
+@@ -3156,11 +3161,22 @@
+ 
+ 	doappenduid = (mailbox.m.myrights & ACL_READ);
+ 
++	r = mboxlist_findstage(mailboxname, stagedir, sizeof(stagedir));
++
+ 	for (i = 0; !r && i < numstage; i++) {
+ 	    if (stage[i]->binary) {
+-		r = message_parse_binary_file(stage[i]->f, &body);
++		snprintf(stagefile, sizeof (stagefile), "%s%s", stagedir,
++			 append_stagename(stage[i]->stage));
++		fileStage = fopen(stagefile, "r");
++		if (!(fileStage)) {
++		    syslog(LOG_ERR, "IOERROR: accessing message file %s: %m", 
++			   stagefile);
++		    r = IMAP_IOERROR;
++		    break;
++		}
++		r = message_parse_binary_file(fileStage, &body);
++		fclose(fileStage); fileStage = NULL;
+ 	    }
+-	    fclose(stage[i]->f); stage[i]->f = NULL;
+ 	    if (!r) {
+ 		r = append_fromstage(&mailbox, &body, stage[i]->stage,
+ 				     stage[i]->internaldate, 
+@@ -3183,10 +3199,10 @@
+     }
+ 
+     /* Cleanup the stage(s) */
++    if (fileStage) fclose(fileStage);
+     while (numstage) {
+ 	curstage = stage[--numstage];
+ 
+-	if (curstage->f != NULL) fclose(curstage->f);
+ 	append_removestage(curstage->stage);
+ 	while (curstage->nflags--) {
+ 	    free(curstage->flag[curstage->nflags]);
--- cyrus-imapd-2.3.7.patch ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:



More information about the freebsd-ports-bugs mailing list