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<<'

2 files changed, 16 insertions, 10 deletions

diff --git a/pop3d.h b/pop3d.h
index 1b70163..fa7de0b 100644
--- a/pop3d.h
+++ b/pop3d.h
@@ -129,7 +129,7 @@ enum state {
 
 struct session {
 	SPLAY_ENTRY(session)	entry;
-	struct imsgev		iev_maildrop;
+	struct imsgev		*iev_maildrop;
 	struct iobuf		iobuf;
 	struct io		io;
 	char			user[ARGLEN];
diff --git a/session.c b/session.c
index e30f50c..8f7622c 100644
--- a/session.c
+++ b/session.c
@@ -94,7 +94,7 @@ static void session_write(struct session *, const char *, size_t);
 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 @@ session_close(struct session *s, int flush)
 
 	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 @@ trans_command(struct session *s, int cmd, char *args)
 	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 @@ trans_command(struct session *s, int cmd, char *args)
 	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 @@ trans_command(struct session *s, int cmd, char *args)
 			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 @@ get_list_all(struct session *s, int uidl)
 {
 	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 @@ get_list(struct session *s, unsigned int i, int uidl)
 
 	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 @@ maildrop_imsgev(struct imsgev *iev, int code, struct imsg *imsg)
 	case IMSGEV_EIMSG:
 		fatal("session: imsgev read/write error");
 		break;
+	case IMSGEV_DONE:
+		free(iev);
+		break;
 	}
 }