rewrite message-passing, no signals needed!

9 files changed, 112 insertions, 199 deletions

diff --git a/.gitignore b/.gitignore
index 0198daa..229fb89 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,5 @@ a.out
 *.bin
 *.o
 *~
+*.d
+netcosm.log
diff --git a/README.md b/README.md
index 67cacd7..13dcb40 100644
--- a/README.md
+++ b/README.md
@@ -6,7 +6,7 @@ features might drift out of existence without prior warning!
 
 ## Installation
 
-### Prerequisities:
+### Prerequisites:
 
 * libgcrypt
 * libev
@@ -48,13 +48,18 @@ follows:
 
 The master polls child pipes for data, and processes any requests it
 finds. The master then writes it's data to the pipes of any children
-involved in the request, signals them with SIGRTMIN+0, and waits for
-all of them to signal back with SIGRTMIN+1. The children read the data
-from the master, and handle it accordingly.
+involved in the request.
 
-TODO: the signal-based ACK framework is really flakey. A better idea
-would be to have the child poll for data from the master at intervals,
-so the whole mess of signal handling can be avoided.
+Child processes that serve connected clients poll for input from the
+master process while waiting for client input. The format of these
+requests is as follows:
+
+    | Request Code | Data (if any) |
+
+Child requests (that is, requests a child sends to the master) are
+very reliable. However, requests from the master process to its child
+processes are not so reliable. The child process may not be polling
+for data, and so would not receive the request.
 
 ## Design Goals
 
diff --git a/src/auth.c b/src/auth.c
index 80a6583..4198302 100644
--- a/src/auth.c
+++ b/src/auth.c
@@ -59,7 +59,6 @@ static char *hash_pass_hex(const char *pass, const char *salt)
     for(unsigned int i = 0; i < hash_len; ++i, ptr += 2)
         snprintf(ptr, 3, "%02x", hash[i]);
     hex[hash_len * 2] = '\0';
-    sig_debugf("hash is %s\n", hex);
 
     gcry_free(hash);
 
@@ -199,19 +198,24 @@ struct userdata_t *auth_check(const char *name2, const char *pass2)
         sig_debugf("auth module: user %s found\n", name2);
         char *new_hash_hex = hash_pass_hex(pass, salt);
 
-        memset(pass, 0, strlen(pass));
-        free(pass);
-
         /* hashes are in HEX to avoid the Trucha bug */
         bool success = !memcmp(new_hash_hex, hash, strlen(hash));
 
         free(new_hash_hex);
 
         if(success)
+        {
+            memset(pass, 0, strlen(pass));
+            free(pass);
+
             return data;
+        }
     }
 
-    sig_debugf("auth failure: username not found\n");
+    sig_debugf("auth failure for user %s\n", name2);
+
+    memset(pass, 0, strlen(pass));
+    free(pass);
 
     /* failure */
     sleep(2);
diff --git a/src/client.c b/src/client.c
index fff08a6..41c9495 100644
--- a/src/client.c
+++ b/src/client.c
@@ -36,6 +36,8 @@ static volatile sig_atomic_t output_locked = 0;
 
 char *current_user = NULL;
 
+void poll_requests(void);
+
 void out_raw(const void *buf, size_t len)
 {
     if(!len)
@@ -130,11 +132,6 @@ void send_master(unsigned char cmd, const void *data, size_t sz)
 
     request_complete = 0;
 
-    sigset_t block, old;
-    sigemptyset(&block);
-    sigaddset(&block, SIGRTMIN);
-    sigprocmask(SIG_BLOCK, &block, &old);
-
     pid_t our_pid = getpid();
 
     if(!data)
@@ -153,10 +150,14 @@ void send_master(unsigned char cmd, const void *data, size_t sz)
 
     write(to_parent, req, 1 + sizeof(pid_t) + sz);
 
-    sigsuspend(&old);
-    sigprocmask(SIG_SETMASK, &old, NULL);
+    /* poll till we get data */
+    struct pollfd pfd;
+    pfd.fd = from_parent;
+    pfd.events = POLLIN;
+
+    poll(&pfd, 1, -1);
 
-    while(!request_complete) usleep(1);
+    poll_requests();
 
     free(req);
 }
@@ -171,22 +172,53 @@ tryagain:
 
     buf = malloc(BUFSZ);
     memset(buf, 0, BUFSZ);
-    if(read(client_fd, buf, BUFSZ - 1) < 0)
-        error("lost connection");
-    buf[BUFSZ - 1] = '\0';
-    if(buf[0] & 0x80)
+
+    /* set of the client fd and the pipe from our parent */
+    struct pollfd fds[2];
+
+    /* order matters here: we first fulfill parent requests, then
+     * handle client data */
+
+    fds[0].fd = from_parent;
+    fds[0].events = POLLIN;
+
+    fds[1].fd = client_fd;
+    fds[1].events = POLLIN;
+
+    while(1)
     {
-        int ret = telnet_handle_command((unsigned char*)buf);
+        poll(fds, ARRAYLEN(fds), -1);
+        for(int i = 0; i < 2; ++i)
+        {
+            if(fds[i].revents == POLLIN)
+            {
+                if(fds[i].fd == from_parent)
+                {
+                    poll_requests();
+                }
+                else if(fds[i].fd == client_fd)
+                {
+                    if(read(client_fd, buf, BUFSZ - 1) < 0)
+                        error("lost connection");
 
-        free(buf);
-        if(ret == TELNET_EXIT)
-            exit(0);
-        goto tryagain;
-    }
+                    buf[BUFSZ - 1] = '\0';
+                    if(buf[0] & 0x80)
+                    {
+                        int ret = telnet_handle_command((unsigned char*)buf);
+
+                        free(buf);
+                        if(ret == TELNET_EXIT)
+                            exit(0);
+                        goto tryagain;
+                    }
 
-    remove_cruft(buf);
+                    remove_cruft(buf);
 
-    return buf;
+                    return buf;
+                }
+            }
+        }
+    }
 }
 
 /* still not encrypted, but a bit more secure than echoing the password! */
@@ -214,32 +246,20 @@ void read_string_max(int fd, char *buf, size_t max)
     buf[max - 1] = '\0';
 }
 
-void sig_rt_0_handler(int s, siginfo_t *info, void *v)
+void poll_requests(void)
 {
-    (void) s;
-    (void) v;
-
     if(!are_child)
         return;
 
-    sig_debugf("Master process sends SIG\n");
-
-    /* we only listen to requests from our parent */
-    if(info->si_pid != getppid())
-    {
-        sig_debugf("Unknown PID sent SIGRTMIN+1\n");
-        return;
-    }
-
-
     reqdata_type = TYPE_NONE;
 
     while(1)
     {
         unsigned char packet[MSG_MAX + 1];
         memset(packet, 0, sizeof(packet));
+
         ssize_t packetlen = read(from_parent, packet, MSG_MAX);
-        sig_debugf("done reading data\n");
+
         unsigned char *data = packet + 1;
         size_t datalen = packetlen - 1;
         packet[MSG_MAX] = '\0';
@@ -249,8 +269,6 @@ void sig_rt_0_handler(int s, siginfo_t *info, void *v)
 
         unsigned char cmd = packet[0];
 
-        sig_debugf("child got code %d\n", cmd);
-
         switch(cmd)
         {
         case REQ_BCASTMSG:
@@ -261,14 +279,11 @@ void sig_rt_0_handler(int s, siginfo_t *info, void *v)
         case REQ_KICK:
         {
             out((char*)data, datalen);
-            union sigval junk = { 0 };
-            /* the master still expects an ACK */
-            sigqueue(getppid(), SIGRTMIN+1, junk);
             exit(EXIT_SUCCESS);
         }
         case REQ_MOVE:
         {
-            bool status = *((bool*)data);
+            int status = *((int*)data);
 
             reqdata_type = TYPE_BOOLEAN;
             returned_reqdata.boolean = status;
@@ -278,14 +293,10 @@ void sig_rt_0_handler(int s, siginfo_t *info, void *v)
         }
         case REQ_GETUSERDATA:
         {
-            sig_debugf("got user data\n");
             if(datalen == sizeof(struct userdata_t))
                 reqdata_type = TYPE_USERDATA;
             else
-            {
-                sig_debugf("failure %d %d\n", datalen, sizeof(struct userdata_t));
                 break;
-            }
 
             struct userdata_t *user = &returned_reqdata.userdata;
             *user = *((struct userdata_t*)data);
@@ -316,25 +327,8 @@ void sig_rt_0_handler(int s, siginfo_t *info, void *v)
         }
     }
 fail:
-    sig_debugf("Client finishes handling request.\n");
 
     request_complete = 1;
-
-    /* signal the master that we're done */
-    union sigval junk = { 0 };
-    sigqueue(getppid(), SIGRTMIN+1, junk);
-}
-
-static void sigpipe_handler(int s)
-{
-    (void) s;
-    union sigval junk = { 0 };
-    /*
-     * necessary in case we get SIGPIPE in our SIGRTMIN+1 handler,
-     * the master expects a response from us
-     */
-    sigqueue(getppid(), SIGRTMIN+1, junk);
-    _exit(0);
 }
 
 void client_change_state(int state)
@@ -414,13 +408,6 @@ void client_main(int fd, struct sockaddr_in *addr, int total, int to, int from)
 
     output_locked = 0;
 
-    struct sigaction sa;
-    sigemptyset(&sa.sa_mask);
-    sa.sa_handler = sigpipe_handler;
-    sa.sa_flags = SA_RESTART;
-    if(sigaction(SIGPIPE, &sa, NULL) < 0)
-        error("sigaction");
-
     telnet_init();
 
     char *ip = inet_ntoa(addr->sin_addr);
diff --git a/src/server.c b/src/server.c
index 50f055f..e651f59 100644
--- a/src/server.c
+++ b/src/server.c
@@ -106,7 +106,9 @@ static void __attribute__((noreturn)) serv_cleanup(void)
     /* kill all our children (usually init claims them and wait()'s
        for them, but not always) */
     struct sigaction sa;
-    sigfillset(&sa.sa_mask); sigaddset(&sa.sa_mask, SIGCHLD);
+    sigfillset(&sa.sa_mask);
+    sigaddset(&sa.sa_mask, SIGCHLD);
+    sa.sa_handler = SIG_IGN;
     sigaction(SIGCHLD, &sa, NULL); /* kill all children */
     void *ptr = child_map, *save;
     do {
@@ -164,29 +166,6 @@ static void init_signals(void)
         error("sigaction");
     if(sigaction(SIGTERM, &sa, NULL) < 0)
         error("sigaction");
-
-    sigemptyset(&sa.sa_mask);
-    sigaddset(&sa.sa_mask, SIGRTMIN+1);
-    sa.sa_sigaction = master_ack_handler;
-    sa.sa_flags = SA_SIGINFO | SA_RESTART;
-    if(sigaction(SIGRTMIN+1, &sa, NULL) < 0)
-        error("sigaction");
-
-    sigemptyset(&sa.sa_mask);
-    sigaddset(&sa.sa_mask, SIGPIPE);
-    sa.sa_handler = SIG_IGN;
-    sa.sa_flags = SA_RESTART;
-    if(sigaction(SIGPIPE, &sa, NULL) < 0)
-        error("sigaction");
-
-    /* we set this now so there's no race condition after a fork() */
-    sigemptyset(&sa.sa_mask);
-    sigaddset(&sa.sa_mask, SIGRTMIN);
-    sigaddset(&sa.sa_mask, SIGPIPE);
-    sa.sa_sigaction = sig_rt_0_handler;
-    sa.sa_flags = SA_RESTART | SA_SIGINFO;
-    if(sigaction(SIGRTMIN, &sa, NULL) < 0)
-        error("sigaction");
 }
 
 static void check_userfile(void)
@@ -251,11 +230,13 @@ static void childreq_cb(EV_P_ ev_io *w, int revents)
 {
     (void) EV_A;
     (void) w;
-    (void) revents;
     /* data from a child's pipe */
-    if(!handle_child_req(w->fd))
+    if(revents & EV_READ)
     {
-        handle_disconnects();
+        if(!handle_child_req(w->fd))
+        {
+            handle_disconnects();
+        }
     }
 }
 
@@ -276,9 +257,9 @@ static void new_connection_cb(EV_P_ ev_io *w, int revents)
     int outpipe [2]; /* parent->child */
 
     if(pipe2(readpipe, O_DIRECT) < 0)
-        error("pipe");
+        error("error creating pipe, need linux kernel >= 3.4");
     if(pipe2(outpipe, O_NONBLOCK | O_DIRECT) < 0)
-        error("pipe");
+        error("error creating pipe, need linux kernel >= 3.4");
 
     pid_t pid = fork();
     if(pid < 0)
diff --git a/src/server_reqs.c b/src/server_reqs.c
index 01e8e16..a17f333 100644
--- a/src/server_reqs.c
+++ b/src/server_reqs.c
@@ -27,6 +27,7 @@ static volatile sig_atomic_t num_acks_wanted, num_acks_recvd, inc_acks = 0;
 static void send_packet(struct child_data *child, unsigned char cmd,
                         void *data, size_t datalen)
 {
+    assert(datalen < MSG_MAX);
     unsigned char pkt[MSG_MAX];
     pkt[0] = cmd;
     if(datalen)
@@ -40,10 +41,6 @@ static void req_pass_msg(unsigned char *data, size_t datalen,
     (void) sender;
 
     send_packet(child, REQ_BCASTMSG, data, datalen);
-
-    union sigval nothing = { 0 };
-    sigqueue(child->pid, SIGRTMIN, nothing);
-    ++num_acks_wanted;
 }
 
 static void req_send_clientinfo(unsigned char *data, size_t datalen,
@@ -81,14 +78,9 @@ static void req_change_state(unsigned char *data, size_t datalen,
 {
     (void) data; (void) datalen; (void) child; (void) sender;
     if(datalen == sizeof(sender->state))
-    {
         sender->state = *((int*)data);
-        debugf("State changed to %d\n", sender->state);
-    }
     else
-    {
         debugf("State data is of the wrong size %*s\n", datalen, data);
-    }
 }
 
 static void req_change_user(unsigned char *data, size_t datalen,
@@ -115,11 +107,7 @@ static void req_kick_client(unsigned char *data, size_t datalen,
     {
         pid_t kicked_pid = *((pid_t*)data);
         if(kicked_pid == child->pid)
-        {
             send_packet(child, REQ_BCASTMSG, data + sizeof(pid_t), datalen - sizeof(pid_t));
-            union sigval nothing = { 0 };
-            sigqueue(child->pid, SIGRTMIN, nothing);
-        }
     }
 }
 
@@ -172,16 +160,14 @@ static void req_move_room(unsigned char *data, size_t datalen, struct child_data
     /* TODO: checking */
     sig_debugf("Moving in direction %d\n", dir);
     room_id new = current->adjacent[dir];
-    bool status;
+    int status = 0;
     if(new != ROOM_NONE)
     {
         child_set_room(sender, new);
-        status = true;
-    }
-    else
-    {
-        status = false;
+        status = 1;
     }
+    sig_debugf("server status: %d\n", status);
+
     send_packet(sender, REQ_MOVE, &status, sizeof(status));
 }
 
@@ -251,19 +237,19 @@ static const struct child_request {
     unsigned char cmd_to_send;
 } requests[] = {
     { REQ_NOP,         false, CHILD_NONE,           NULL,                NULL,              REQ_NOP         },
-    { REQ_BCASTMSG,    true,  CHILD_ALL,            req_pass_msg,        NULL,              REQ_BCASTMSG    },
-    { REQ_LISTCLIENTS, false, CHILD_ALL,            req_send_clientinfo, req_send_geninfo,  REQ_BCASTMSG    },
+    { REQ_BCASTMSG,    true,  CHILD_ALL,            req_pass_msg,        NULL,              REQ_NOP         },
+    { REQ_LISTCLIENTS, false, CHILD_ALL,            req_send_clientinfo, req_send_geninfo,  REQ_NOP         },
     { REQ_CHANGESTATE, true,  CHILD_SENDER,         req_change_state,    NULL,              REQ_NOP         },
     { REQ_CHANGEUSER,  true,  CHILD_SENDER,         req_change_user,     NULL,              REQ_NOP         },
     { REQ_KICK,        true,  CHILD_ALL,            req_kick_client,     NULL,              REQ_NOP         },
     { REQ_WAIT,        false, CHILD_NONE,           NULL,                req_wait,          REQ_NOP         },
-    { REQ_GETROOMDESC, false, CHILD_NONE,           NULL,                req_send_desc,     REQ_BCASTMSG    },
-    { REQ_GETROOMNAME, false, CHILD_NONE,           NULL,                req_send_roomname, REQ_BCASTMSG    },
+    { REQ_GETROOMDESC, false, CHILD_NONE,           NULL,                req_send_desc,     REQ_NOP         },
+    { REQ_GETROOMNAME, false, CHILD_NONE,           NULL,                req_send_roomname, REQ_NOP         },
     { REQ_SETROOM,     true,  CHILD_NONE,           NULL,                req_set_room,      REQ_NOP         },
-    { REQ_MOVE,        true,  CHILD_NONE,           NULL,                req_move_room,     REQ_MOVE        },
-    { REQ_GETUSERDATA, true,  CHILD_NONE,           NULL,                req_send_user,     REQ_GETUSERDATA },
-    { REQ_DELUSERDATA, true,  CHILD_NONE,           NULL,                req_del_user,      REQ_DELUSERDATA },
-    { REQ_ADDUSERDATA, true,  CHILD_NONE,           NULL,                req_add_user,      REQ_ADDUSERDATA },
+    { REQ_MOVE,        true,  CHILD_NONE,           NULL,                req_move_room,     REQ_NOP         },
+    { REQ_GETUSERDATA, true,  CHILD_NONE,           NULL,                req_send_user,     REQ_NOP         },
+    { REQ_DELUSERDATA, true,  CHILD_NONE,           NULL,                req_del_user,      REQ_NOP         },
+    { REQ_ADDUSERDATA, true,  CHILD_NONE,           NULL,                req_add_user,      REQ_NOP         },
     //{ REQ_ROOMMSG,     true,  CHILD_ALL,            req_send_room_msg,   NULL,              REQ_BCASTMSG },
 };
 
@@ -308,6 +294,9 @@ bool handle_child_req(int in_fd)
 
     ssize_t packet_len = read(in_fd, packet, MSG_MAX);
 
+    if(packet_len <= 0)
+        goto fail;
+
     pid_t sender_pid;
     memcpy(&sender_pid, packet, sizeof(pid_t));
     sig_debugf("Got request from PID %d\n", sender_pid);
@@ -327,16 +316,6 @@ bool handle_child_req(int in_fd)
 
     struct child_request *req = hash_lookup(request_map, &cmd);
 
-    sigset_t old, block;
-
-    sigemptyset(&block);
-    sigaddset(&block, SIGRTMIN+1);
-    sigprocmask(SIG_BLOCK, &block, &old);
-
-    num_acks_wanted = 1;
-    num_acks_recvd  = 0;
-    inc_acks = 1;
-
     if(!req)
     {
         sig_debugf("Unknown request.\n");
@@ -382,47 +361,14 @@ bool handle_child_req(int in_fd)
 
 finish:
 
-    //if(req)
-    //{
-    //    send_packet(sender, req->cmd_to_send, NULL, 0);
-    //}
-
-    if(req && req->finalize)
-        req->finalize(data, datalen, sender);
-
-    union sigval junk = { 0 };
-    if(sender)
-        sigqueue(sender->pid, SIGRTMIN, junk);
-    else
-        sig_debugf("Unknown PID sent request.\n");
-
-    /* 5 ms */
-#define ACK_TIMEOUT 5000
-
-    struct timespec timeout;
-    timeout.tv_sec = 0;
-    timeout.tv_nsec = ACK_TIMEOUT;
-
-    while(num_acks_recvd < MIN(num_clients,num_acks_wanted))
+    if(req)
     {
-        if(sigtimedwait(&old, NULL, &timeout) < 0 && errno == EAGAIN)
-            break;
+        send_packet(sender, req->cmd_to_send, NULL, 0);
     }
 
-    inc_acks = 0;
-
-    sigprocmask(SIG_SETMASK, &old, NULL);
+    if(req && req->finalize)
+        req->finalize(data, datalen, sender);
 
 fail:
     return true;
 }
-
-void master_ack_handler(int s, siginfo_t *info, void *v)
-{
-    (void) s;
-    (void) v;
-    if(inc_acks && hash_lookup(child_map, &info->si_pid))
-    {
-        ++num_acks_recvd;
-    }
-}
diff --git a/src/server_reqs.h b/src/server_reqs.h
index 9502c11..fcd835b 100644
--- a/src/server_reqs.h
+++ b/src/server_reqs.h
@@ -19,7 +19,7 @@
 /* child<->master commands */
 /* children might not implement all of these */
 /* meanings might be different for the server and child, see comments */
-#define REQ_NOP               0 /* server, child: do nothing */
+#define REQ_NOP               0 /* server, child: do nothing (used for acknowledgement) */
 #define REQ_BCASTMSG          1 /* server: broadcast text; child: print following text */
 #define REQ_LISTCLIENTS       2 /* server: list childs */
 #define REQ_CHANGESTATE       3 /* server: change child state flag */
diff --git a/src/util.c b/src/util.c
index 6d2d184..99a70c5 100644
--- a/src/util.c
+++ b/src/util.c
@@ -33,18 +33,8 @@ void debugf_real(const char *func, int line, const char *file, const char *fmt,
     (void) func;
     (void) line;
     (void) file;
-    static int fd = -1;
-    if(fd < 0)
-        fd = open(LOGFILE, O_APPEND | O_WRONLY | O_CREAT, 0600);
-    if(fd < 0)
-        error("unknown");
+
     int len;
-#if 0
-    char *prefix;
-    len = asprintf(&prefix, "%s:%s:%d: ", func, file, line);
-    write(STDOUT_FILENO, prefix, len);
-    free(prefix);
-#endif
 
     va_list ap;
     va_start(ap, fmt);
@@ -53,8 +43,6 @@ void debugf_real(const char *func, int line, const char *file, const char *fmt,
     len = vasprintf(&buf, fmt, ap);
 
     write(STDOUT_FILENO, buf, len);
-    write(fd, buf, len);
-    fflush(fdopen(fd, "a"));
 
     free(buf);
 
diff --git a/worlds/test.c b/worlds/test.c
index 3bb9287..770576e 100644
--- a/worlds/test.c
+++ b/worlds/test.c
@@ -24,7 +24,7 @@ const struct roomdata_t netcosm_world[] = {
     {
         "beride_square_n_statue",
         "King Ajax IV Statue",
-        "Your path is blocked by an enormous bronze statue. A plaque on the pedestal reads,\n\nKing Ajax IV\n\n182 - 238 A.B.A. To the south is the Beride Town Square.",
+        "Your path is blocked by an enormous bronze statue. A plaque on the pedestal reads,\n\nKing Ajax IV\n\n182 - 238 A.B.A.\nTo the south is the Beride Town Square.",
         { NONE_N, NONE_NE, NONE_E, NONE_SE, "world_start", NONE_SW, NONE_W, NONE_NW, NONE_UP, NONE_DN, NONE_IN, NONE_OT },
         NULL,
         NULL,