Skip to content

Commit

Permalink
idle: don't use signals, use AF_UNIX dgrams
Browse files Browse the repository at this point in the history 
Communications back from idled to imapds are via a message sent on the
AF_UNIX socket.  The IDLE command is implemented as a select() loop, and
there's absolutely nothing that needs to be done in signal handler
context.  Best of all, no more unexpected delivery of SIGUSR1 or
SIGUSER2, assassinating innocent bystander processes.
  • Loading branch information
Greg Banks committed Mar 27, 2012
1 parent 720f676 commit 17eb391
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 134 deletions.
129 changes: 66 additions & 63 deletions imap/idle.c
View file
Expand Up @@ -55,16 +55,14 @@
#endif
#include <signal.h>
#include <string.h>
#include <errno.h>

#include "idle.h"
#include "idlemsg.h"
#include "global.h"

const char *idle_method_desc = "no";

/* function to report mailbox updates to the client */
static idle_updateproc_t *idle_update = NULL;

/* how often to poll the mailbox */
static time_t idle_period = -1;
static int idle_started = 0;
Expand All @@ -79,7 +77,6 @@ static int idle_send_msg(int which, const char *mboxname)

/* fill the structure */
msg.which = which;
msg.pid = getpid();
strncpy(msg.mboxname, mboxname ? mboxname : ".", sizeof(msg.mboxname));

/* send */
Expand Down Expand Up @@ -158,82 +155,88 @@ int idle_enabled(void)
}
}

static void idle_handler(int sig)
void idle_start(const char *mboxname)
{
/* ignore the signals, unless the server has started idling */
if (!idle_started) return;

switch (sig) {
case SIGUSR1:
idle_update(IDLE_MAILBOX);
break;
case SIGUSR2:
idle_update(IDLE_ALERT);
break;
case SIGALRM:
idle_update(IDLE_MAILBOX|IDLE_ALERT);
alarm(idle_period);
break;
}
idle_started = 1;

/* Tell idled that we're idling. It doesn't
* matter if it fails, we'll still poll */
idle_send_msg(IDLE_MSG_INIT, mboxname);
}

int idle_init(idle_updateproc_t *proc)
int idle_wait(int otherfd)
{
struct sigaction action;

idle_update = proc;

/* We don't want recursive calls to idle_update() */
sigemptyset(&action.sa_mask);
sigaddset(&action.sa_mask, SIGUSR1);
sigaddset(&action.sa_mask, SIGUSR2);
action.sa_flags = 0;
#ifdef SA_RESTART
action.sa_flags |= SA_RESTART;
#endif
action.sa_handler = idle_handler;

/* Setup the signal handlers */
if ((sigaction(SIGUSR1, &action, NULL) < 0) ||
(sigaction(SIGUSR2, &action, NULL) < 0) ||
(sigaction(SIGALRM, &action, NULL) < 0)) {
syslog(LOG_ERR, "sigaction: %m");

/* Cancel receiving signals */
idle_done(NULL);
int s = idle_get_sock();
fd_set rfds;
int maxfd;
struct timeval timeout;
int r;
int flags = 0;

if (!idle_started)
return 0;
}

return 1;
}
do {
FD_ZERO(&rfds);
maxfd = -1;
if (s >= 0) {
FD_SET(s, &rfds);
maxfd = MAX(maxfd, s);
}
if (otherfd >= 0) {
FD_SET(otherfd, &rfds);
maxfd = MAX(maxfd, otherfd);
}

void idle_start(const char *mboxname)
{
idle_started = 1;
/* Note: it's technically valid for there to be no fds to listen
* to, in the case where @otherfd is passed as -1 and we failed
* to talk to idled. It shouldn't happen though as we're always
* called with a valid otherfd. */

/* TODO: this is wrong, we actually want a rolling period */
timeout.tv_sec = idle_period;
timeout.tv_usec = 0;

r = select(maxfd+1, &rfds, NULL, NULL, &timeout);
if (r < 0) {
if (errno == EAGAIN || errno == EINTR)
continue;
syslog(LOG_ERR, "select: %m");
return 0;
}
if (r == 0) {
/* timeout */
flags |= IDLE_MAILBOX|IDLE_ALERT;
}
if (r > 0 && FD_ISSET(idle_get_sock(), &rfds)) {
struct sockaddr_un from;
idle_message_t msg;

if (idle_recv(&from, &msg)) {
switch (msg.which) {
case IDLE_MSG_NOTIFY:
flags |= IDLE_MAILBOX;
break;
case IDLE_MSG_ALERT:
flags |= IDLE_ALERT;
break;
}
}
}
if (r > 0 && otherfd >= 0 && FD_ISSET(otherfd, &rfds))
flags |= IDLE_INPUT;
} while (!flags);

/* Tell idled that we're idling */
if (!idle_send_msg(IDLE_MSG_INIT, mboxname)) {
/* otherwise, we'll poll with SIGALRM */
alarm(idle_period);
}
return flags;
}

void idle_done(const char *mboxname)
{
/* Tell idled that we're done idling */
idle_send_msg(IDLE_MSG_DONE, mboxname);

/* Cancel alarm */
alarm(0);

/* Remove the signal handlers */
signal(SIGUSR1, SIG_IGN);
signal(SIGUSR2, SIG_IGN);
signal(SIGALRM, SIG_IGN);

/* close the AF_UNIX socket */
idle_done_sock();

idle_update = NULL;
idle_started = 0;
}
25 changes: 18 additions & 7 deletions imap/idle.h
View file
Expand Up @@ -49,8 +49,15 @@
extern const char *idle_method_desc;

typedef enum {
/* something noteworthy may have happened to the mailbox,
* e.g. a delivery, so it needs to be checked */
IDLE_MAILBOX = 0x1,
IDLE_ALERT = 0x2
/* the shutdownfile may have been written, needing an ALERT reponse
* to be sent to any IMAP clients */
IDLE_ALERT = 0x2,
/* input was detected on the @otherfd, probably because the IMAP
* client cancelled the IDLE */
IDLE_INPUT = 0x4
} idle_flags_t;

typedef void idle_updateproc_t(idle_flags_t flags);
Expand All @@ -59,15 +66,19 @@ typedef void idle_updateproc_t(idle_flags_t flags);
/* Is IDLE enabled? Can also do initial setup, if necessary */
int idle_enabled(void);

/* Setup for IDLE.
* 'proc' is a pointer to a function which reports mailbox updates and/or
* ALERTs to the client.
*/
int idle_init(idle_updateproc_t *proc);

/* Start IDLEing on 'mailbox'. */
void idle_start(const char *mboxname);

/* Wait for something to happen while IDLEing. @otherfd is a file
* descriptor on which to wait for input; presumably this will be the
* fd of the main protstream from the IMAP client. Returns a mask of
* flags indicating what if anything happened, see idle_flags_t, or 0
* on error. If idled is disabled or was not contacted, we fall back
* to polling mode and return the flags IDLE_MAILBOX and IDLE_INPUT
* periodically.
*/
int idle_wait(int otherfd);

/* Cleanup when IDLE is completed. */
void idle_done(const char *mboxname);

Expand Down
80 changes: 49 additions & 31 deletions imap/idled.c
View file
Expand Up @@ -76,7 +76,7 @@ static time_t idle_timeout;
static volatile sig_atomic_t sigquit = 0;

struct ientry {
pid_t pid;
struct sockaddr_un remote;
time_t itime;
struct ientry *next;
};
Expand Down Expand Up @@ -105,19 +105,20 @@ static int mbox_count_cb(void *rockp,
return 0;
}

/* remove pid from list of those idling on mboxname */
static void remove_ientry(const char *mboxname, pid_t pid)
/* remove an ientry from list of those idling on mboxname */
static void remove_ientry(const char *mboxname,
const struct sockaddr_un *remote)
{
struct ientry *t, *p = NULL;

t = (struct ientry *) hash_lookup(mboxname, &itable);
while (t && t->pid != pid) {
while (t && memcmp(&t->remote, remote, sizeof(*remote))) {
p = t;
t = t->next;
}
if (t) {
if (!p) {
/* first pid in the linked list */
/* first ientry in the linked list */

p = t->next; /* remove node */

Expand All @@ -126,28 +127,30 @@ static void remove_ientry(const char *mboxname, pid_t pid)
hash_insert(mboxname, p, &itable);
}
else {
/* not the first pid in the linked list */
/* not the first ientry in the linked list */

p->next = t->next; /* remove node */
}
free(t);
}
}

static void process_message(idle_message_t *msg)


static void process_message(struct sockaddr_un *remote, idle_message_t *msg)
{
struct ientry *t, *n;

switch (msg->which) {
case IDLE_MSG_INIT:
if (verbose || debugmode)
syslog(LOG_DEBUG, "imapd[%ld]: IDLE_MSG_INIT '%s'\n",
msg->pid, msg->mboxname);
syslog(LOG_DEBUG, "imapd[%s]: IDLE_MSG_INIT '%s'\n",
idle_id_from_addr(remote), msg->mboxname);

/* add pid to list of those idling on mboxname */
/* add an ientry to list of those idling on mboxname */
t = (struct ientry *) hash_lookup(msg->mboxname, &itable);
n = (struct ientry *) xzmalloc(sizeof(struct ientry));
n->pid = msg->pid;
n->remote = *remote;
n->itime = time(NULL);
n->next = t;
hash_insert(msg->mboxname, n, &itable);
Expand All @@ -157,37 +160,40 @@ static void process_message(idle_message_t *msg)
if (verbose || debugmode)
syslog(LOG_DEBUG, "IDLE_MSG_NOTIFY '%s'\n", msg->mboxname);

/* send a message to all pids idling on mboxname */
/* send a message to all clients idling on mboxname */
t = (struct ientry *) hash_lookup(msg->mboxname, &itable);
while (t) {
for ( ; t ; t = n) {
n = t->next;
if ((t->itime + idle_timeout) < time(NULL)) {
/* This process has been idling for longer than the timeout
* period, so it probably died. Remove it from the list.
*/
if (verbose || debugmode)
syslog(LOG_DEBUG, " TIMEOUT %d\n", t->pid);
syslog(LOG_DEBUG, " TIMEOUT %s\n", idle_id_from_addr(&t->remote));

n = t;
t = t->next;
remove_ientry(msg->mboxname, n->pid);
remove_ientry(msg->mboxname, &t->remote);
}
else { /* signal process to update */
if (verbose || debugmode)
syslog(LOG_DEBUG, " SIGUSR1 %d\n", t->pid);

kill(t->pid, SIGUSR1);
t = t->next;
syslog(LOG_DEBUG, " fwd NOTIFY %s\n", idle_id_from_addr(&t->remote));

/* forward the received msg onto our clients */
if (!idle_send(&t->remote, msg)) {
if (verbose || debugmode)
syslog(LOG_DEBUG, " forgetting %s\n", idle_id_from_addr(&t->remote));
remove_ientry(msg->mboxname, &t->remote);
}
}
}
break;

case IDLE_MSG_DONE:
if (verbose || debugmode)
syslog(LOG_DEBUG, "imapd[%ld]: IDLE_MSG_DONE '%s'\n",
msg->pid, msg->mboxname);
syslog(LOG_DEBUG, "imapd[%s]: IDLE_MSG_DONE '%s'\n",
idle_id_from_addr(remote), msg->mboxname);

/* remove pid from list of those idling on mboxname */
remove_ientry(msg->mboxname, msg->pid);
/* remove client from list of those idling on mboxname */
remove_ientry(msg->mboxname, remote);
break;

case IDLE_MSG_NOOP:
Expand All @@ -199,19 +205,31 @@ static void process_message(idle_message_t *msg)
}
}

static void send_alert(const char *key __attribute__((unused)),

static void send_alert(const char *key,
void *data,
void *rock __attribute__((unused)))
{
struct ientry *t = (struct ientry *) data;
struct ientry *n;
idle_message_t msg;

msg.which = IDLE_MSG_ALERT;
strncpy(msg.mboxname, ".", sizeof(msg.mboxname));


for ( ; t ; t = n) {
n = t->next;

while (t) {
/* signal process to check ALERTs */
if (verbose || debugmode)
syslog(LOG_DEBUG, " SIGUSR2 %d\n", t->pid);
kill(t->pid, SIGUSR2);
syslog(LOG_DEBUG, " ALERT %s\n", idle_id_from_addr(&t->remote));

t = t->next;
if (!idle_send(&t->remote, &msg)) {
if (verbose || debugmode)
syslog(LOG_DEBUG, " forgetting %s\n", idle_id_from_addr(&t->remote));
remove_ientry(key, &t->remote);
}
}
}

Expand Down Expand Up @@ -355,7 +373,7 @@ int main(int argc, char **argv)
idle_message_t msg;

if (idle_recv(&from, &msg))
process_message(&msg);
process_message(&from, &msg);
}

}
Expand Down

0 comments on commit 17eb391

Please sign in to comment.