# HG changeset patch # User Sunil Nimmagadda # Date 1409483885 -18030 # Node ID e3dccf824f912dae798e67b7172ea370a3cacb48 # Parent a52328aa693efaff23ae411c4b22c718c54f3b6b 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<<' diff -r a52328aa693e -r e3dccf824f91 pop3d.h --- 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]; diff -r a52328aa693e -r e3dccf824f91 session.c --- 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; } }