Disengaging imsgev is tricky. First imsgev_clear as we know there
authorSunil Nimmagadda <sunil@nimmagadda.net>
Sun, 31 Aug 2014 16:18:35 +0500
changeset 37 e3dccf824f91
parent 34 a52328aa693e
child 38 058e5ad9ba79
Disengaging imsgev is tricky. First imsgev_clear as we know there isn't anything left to be written. Signal imsgev termination by setting iev->terminate = 1(Mind that we are setting it from with the callback). As imsgev would again callback dispatch with IMSGEV_DONE, we cannot free imsgev yet. So iev_maildrop need to exist beyond lifetime of session until IMSGEV_DONE. Allocating it seperately from session. imsgev_close is avoided as it schedules another EV_WRITE which is not needed in our case. This fixes a crash observed consistently with a little perl script to simulate concurrent sessions. Thanks MALLOC_OPTIONS='SFG<<'
pop3d.h
session.c
--- a/pop3d.h	Wed Aug 27 16:21:31 2014 +0500
+++ b/pop3d.h	Sun Aug 31 16:18:35 2014 +0500
@@ -129,7 +129,7 @@
 
 struct session {
 	SPLAY_ENTRY(session)	entry;
-	struct imsgev		iev_maildrop;
+	struct imsgev		*iev_maildrop;
 	struct iobuf		iobuf;
 	struct io		io;
 	char			user[ARGLEN];
--- a/session.c	Wed Aug 27 16:21:31 2014 +0500
+++ b/session.c	Sun Aug 31 16:18:35 2014 +0500
@@ -94,7 +94,7 @@
 static const char *strstate(enum state);
 
 struct session_tree	sessions;
-static int		_pop3_debug = 1;
+static int		_pop3_debug = 0;
 
 void
 session_init(struct listener *l, int fd)
@@ -147,7 +147,8 @@
 
 	io_clear(&entry->io);
 	iobuf_clear(&entry->iobuf);
-	imsgev_clear(&entry->iev_maildrop);
+	imsgev_clear(entry->iev_maildrop);
+	entry->iev_maildrop->terminate = 1;
 	logit(LOG_INFO, "%u: session closed", entry->id);
 	free(entry);
 }
@@ -373,7 +374,7 @@
 	case CMD_RETR:
 		if (!get_index(s, args, &retr_req.idx))
 			break;
-		imsgev_xcompose(&s->iev_maildrop, IMSG_MAILDROP_RETR,
+		imsgev_xcompose(s->iev_maildrop, IMSG_MAILDROP_RETR,
 		    s->id, 0, -1, &retr_req, sizeof(retr_req), "trans_command");
 		return;
 	case CMD_NOOP:
@@ -382,11 +383,11 @@
 	case CMD_DELE:
 		if (!get_index(s, args, &idx))
 			break;
-		imsgev_xcompose(&s->iev_maildrop, IMSG_MAILDROP_DELE,
+		imsgev_xcompose(s->iev_maildrop, IMSG_MAILDROP_DELE,
 		    s->id, 0, -1, &idx, sizeof(idx), "trans_command");
 		return;
 	case CMD_RSET:
-		imsgev_xcompose(&s->iev_maildrop, IMSG_MAILDROP_RSET,
+		imsgev_xcompose(s->iev_maildrop, IMSG_MAILDROP_RSET,
 		    s->id, 0, -1, NULL, 0, "trans_command");
 		return;
 	case CMD_UIDL:
@@ -401,7 +402,7 @@
 			get_list_all(s, uidl);
 		return;
 	case CMD_QUIT:
-		imsgev_xcompose(&s->iev_maildrop, IMSG_MAILDROP_UPDATE,
+		imsgev_xcompose(s->iev_maildrop, IMSG_MAILDROP_UPDATE,
 		    s->id, 0, -1, NULL, 0, "trans_command");
 		session_set_state(s, UPDATE);
 		return;
@@ -418,7 +419,7 @@
 {
 	io_pause(&s->io, IO_PAUSE_IN);
 	session_reply(s, "+OK");
-	imsgev_xcompose(&s->iev_maildrop, IMSG_MAILDROP_LISTALL,
+	imsgev_xcompose(s->iev_maildrop, IMSG_MAILDROP_LISTALL,
 	    s->id, 0, -1, &uidl, sizeof(uidl), "list_all");
 }
 
@@ -429,14 +430,16 @@
 
 	req.idx = i;
 	req.uidl = uidl;
-	imsgev_xcompose(&s->iev_maildrop, IMSG_MAILDROP_LIST,
+	imsgev_xcompose(s->iev_maildrop, IMSG_MAILDROP_LIST,
 	    s->id, 0, -1, &req, sizeof(req), "list");
 }
 
 void
 session_imsgev_init(struct session *s, int fd)
 {
-	imsgev_init(&s->iev_maildrop, fd, NULL, maildrop_imsgev, needfd);
+	s->iev_maildrop = xcalloc(1, sizeof(struct imsgev),
+	    "session_imsgev_init");
+	imsgev_init(s->iev_maildrop, fd, NULL, maildrop_imsgev, needfd);
 }
 
 static void
@@ -486,6 +489,9 @@
 	case IMSGEV_EIMSG:
 		fatal("session: imsgev read/write error");
 		break;
+	case IMSGEV_DONE:
+		free(iev);
+		break;
 	}
 }