better synchronization, less weird behavior

5 files changed, 71 insertions, 61 deletions

diff --git a/src/client.c b/src/client.c
index 41c9495..464fca8 100644
--- a/src/client.c
+++ b/src/client.c
@@ -36,7 +36,7 @@ static volatile sig_atomic_t output_locked = 0;
 
 char *current_user = NULL;
 
-void poll_requests(void);
+bool poll_requests(void);
 
 void out_raw(const void *buf, size_t len)
 {
@@ -157,7 +157,7 @@ void send_master(unsigned char cmd, const void *data, size_t sz)
 
     poll(&pfd, 1, -1);
 
-    poll_requests();
+    while(poll_requests());
 
     free(req);
 }
@@ -246,12 +246,12 @@ void read_string_max(int fd, char *buf, size_t max)
     buf[max - 1] = '\0';
 }
 
-void poll_requests(void)
+bool poll_requests(void)
 {
     if(!are_child)
-        return;
+        return false;
 
-    reqdata_type = TYPE_NONE;
+    bool got_cmd = false;
 
     while(1)
     {
@@ -267,6 +267,8 @@ void poll_requests(void)
         if(packetlen <= 0)
             goto fail;
 
+        got_cmd = true;
+
         unsigned char cmd = packet[0];
 
         switch(cmd)
@@ -316,11 +318,14 @@ void poll_requests(void)
         }
         case REQ_NOP:
             break;
-        case REQ_PRINT_NL:
+        case REQ_PRINTNEWLINE:
         {
             out("\n");
             break;
         }
+        case REQ_ALLDONE:
+            request_complete = 1;
+            break;
         default:
             sig_debugf("WARNING: client process received unknown code %d\n", cmd);
             break;
@@ -328,7 +333,7 @@ void poll_requests(void)
     }
 fail:
 
-    request_complete = 1;
+    return got_cmd;
 }
 
 void client_change_state(int state)
diff --git a/src/server.c b/src/server.c
index e651f59..90a313f 100644
--- a/src/server.c
+++ b/src/server.c
@@ -25,7 +25,7 @@
 #include "util.h"
 
 #define PORT 1234
-#define BACKLOG 100
+#define BACKLOG 512
 
 /* global data */
 bool are_child = false;
@@ -61,7 +61,7 @@ static void free_child_data(void *ptr)
     }
     if(child->io_watcher)
     {
-        ev_io_stop(child->loop, child->io_watcher);
+        ev_io_stop(EV_DEFAULT_ child->io_watcher);
 
         free(child->io_watcher);
         child->io_watcher = NULL;
@@ -87,10 +87,11 @@ static void handle_disconnects(void)
     errno = saved_errno;
 }
 
-static void sigchld_handler(int s)
+volatile sig_atomic_t reap_children = 0;
+
+static void sigchld_handler(int sig)
 {
-    (void) s;
-    handle_disconnects();
+    reap_children = 1;
 }
 
 static void handle_client(int fd, struct sockaddr_in *addr,
@@ -147,27 +148,6 @@ static void __attribute__((noreturn)) sigint_handler(int s)
     serv_cleanup();
 }
 
-static void init_signals(void)
-{
-    struct sigaction sa;
-
-    sigemptyset(&sa.sa_mask);
-    sigaddset(&sa.sa_mask, SIGCHLD);
-    sa.sa_handler = sigchld_handler;
-    sa.sa_flags = SA_RESTART;
-    if(sigaction(SIGCHLD, &sa, NULL) < 0)
-        error("sigaction");
-
-    sigemptyset(&sa.sa_mask);
-    sigaddset(&sa.sa_mask, SIGINT);
-    sa.sa_handler = sigint_handler;
-    sa.sa_flags = SA_RESTART;
-    if(sigaction(SIGINT, &sa, NULL) < 0)
-        error("sigaction");
-    if(sigaction(SIGTERM, &sa, NULL) < 0)
-        error("sigaction");
-}
-
 static void check_userfile(void)
 {
     if(access(USERFILE, F_OK) < 0 || userdb_size() == 0)
@@ -230,6 +210,8 @@ static void childreq_cb(EV_P_ ev_io *w, int revents)
 {
     (void) EV_A;
     (void) w;
+    if(reap_children)
+        handle_disconnects();
     /* data from a child's pipe */
     if(revents & EV_READ)
     {
@@ -314,8 +296,6 @@ static void new_connection_cb(EV_P_ ev_io *w, int revents)
         ev_io_start(EV_A_ new_io_watcher);
         new->io_watcher = new_io_watcher;
 
-        new->loop = loop;
-
         pid_t *pidbuf = malloc(sizeof(pid_t));
         *pidbuf = pid;
 
@@ -323,6 +303,36 @@ static void new_connection_cb(EV_P_ ev_io *w, int revents)
     }
 }
 
+static void init_signals(void)
+{
+    struct sigaction sa;
+
+    sigemptyset(&sa.sa_mask);
+    sigaddset(&sa.sa_mask, SIGINT);
+    sa.sa_handler = sigint_handler;
+    sa.sa_flags = SA_RESTART;
+    if(sigaction(SIGINT, &sa, NULL) < 0)
+        error("sigaction");
+    if(sigaction(SIGTERM, &sa, NULL) < 0)
+        error("sigaction");
+
+    sigemptyset(&sa.sa_mask);
+    sa.sa_handler = SIG_IGN;
+    sa.sa_flags = 0;
+    if(sigaction(SIGPIPE, &sa, NULL) < 0)
+        error("sigaction");
+
+    /* libev's default SIGCHLD handler exhibits some really strange
+     * behavior, which we don't like, so we use our own ;) */
+
+    sigemptyset(&sa.sa_mask);
+    sigaddset(&sa.sa_mask, SIGCHLD);
+    sa.sa_handler = sigchld_handler;
+    sa.sa_flags = SA_RESTART;
+    if(sigaction(SIGCHLD, &sa, NULL) < 0)
+        error("sigaction");
+}
+
 int main(int argc, char *argv[])
 {
     debugf("*** Starting NetCosm "NETCOSM_VERSION" (libev %d.%d, libgcrypt "GCRYPT_VERSION") ***\n",
@@ -358,7 +368,7 @@ int main(int argc, char *argv[])
 
     struct ev_loop *loop = EV_DEFAULT;
 
-    /* set up signal handlers AFTER creating the default loop, because it will grab SIGCHLD */
+    /* we initialize signals after creating the default event loop */
     init_signals();
 
     ev_io server_watcher;
diff --git a/src/server.h b/src/server.h
index 3117955..21f2076 100644
--- a/src/server.h
+++ b/src/server.h
@@ -37,7 +37,6 @@ struct child_data {
 
     ev_io *io_watcher;
     ev_child *sigchld_watcher;
-    struct ev_loop *loop;
 
     struct in_addr addr;
 };
diff --git a/src/server_reqs.c b/src/server_reqs.c
index a17f333..0aa8223 100644
--- a/src/server_reqs.c
+++ b/src/server_reqs.c
@@ -22,8 +22,6 @@
 #include "server.h"
 #include "userdb.h"
 
-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)
 {
@@ -123,7 +121,7 @@ static void req_send_desc(unsigned char *data, size_t datalen, struct child_data
     struct room_t *room = room_get(sender->room);
     send_packet(sender, REQ_BCASTMSG, room->data.desc, strlen(room->data.desc));
 
-    send_packet(sender, REQ_PRINT_NL, NULL, 0);
+    send_packet(sender, REQ_PRINTNEWLINE, NULL, 0);
 }
 
 static void req_send_roomname(unsigned char *data, size_t datalen, struct child_data *sender)
@@ -132,7 +130,7 @@ static void req_send_roomname(unsigned char *data, size_t datalen, struct child_
     struct room_t *room = room_get(sender->room);
     send_packet(sender, REQ_BCASTMSG, room->data.name, strlen(room->data.name));
 
-    send_packet(sender, REQ_PRINT_NL, NULL, 0);
+    send_packet(sender, REQ_PRINTNEWLINE, NULL, 0);
 }
 
 static void child_set_room(struct child_data *child, room_id id)
@@ -232,25 +230,22 @@ static const struct child_request {
                          struct child_data *sender, struct child_data *child);
 
     void (*finalize)(unsigned char *data, size_t len, struct child_data *sender);
-
-    /* byte to write back to the sender */
-    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_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_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_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 },
+    { REQ_NOP,         false, CHILD_NONE,           NULL,                NULL,              },
+    { REQ_BCASTMSG,    true,  CHILD_ALL,            req_pass_msg,        NULL,              },
+    { REQ_LISTCLIENTS, false, CHILD_ALL,            req_send_clientinfo, req_send_geninfo,  },
+    { REQ_CHANGESTATE, true,  CHILD_SENDER,         req_change_state,    NULL,              },
+    { REQ_CHANGEUSER,  true,  CHILD_SENDER,         req_change_user,     NULL,              },
+    { REQ_KICK,        true,  CHILD_ALL,            req_kick_client,     NULL,              },
+    { REQ_WAIT,        false, CHILD_NONE,           NULL,                req_wait,          },
+    { REQ_GETROOMDESC, false, CHILD_NONE,           NULL,                req_send_desc,     },
+    { REQ_GETROOMNAME, false, CHILD_NONE,           NULL,                req_send_roomname, },
+    { REQ_SETROOM,     true,  CHILD_NONE,           NULL,                req_set_room,      },
+    { REQ_MOVE,        true,  CHILD_NONE,           NULL,                req_move_room,     },
+    { REQ_GETUSERDATA, true,  CHILD_NONE,           NULL,                req_send_user,     },
+    { REQ_DELUSERDATA, true,  CHILD_NONE,           NULL,                req_del_user,      },
+    { REQ_ADDUSERDATA, true,  CHILD_NONE,           NULL,                req_add_user,      },
+    //{ REQ_ROOMMSG,     true,  CHILD_ALL,            req_send_room_msg,   NULL,           },
 };
 
 static SIMP_HASH(unsigned char, uchar_hash);
@@ -363,7 +358,7 @@ finish:
 
     if(req)
     {
-        send_packet(sender, req->cmd_to_send, NULL, 0);
+        send_packet(sender, REQ_ALLDONE, NULL, 0);
     }
 
     if(req && req->finalize)
diff --git a/src/server_reqs.h b/src/server_reqs.h
index fcd835b..06e3f46 100644
--- a/src/server_reqs.h
+++ b/src/server_reqs.h
@@ -35,7 +35,8 @@
 #define REQ_GETUSERDATA       13 /* server: send user data; child: get user data */
 #define REQ_DELUSERDATA       14 /* server: delete user data; child: success/failure */
 #define REQ_ADDUSERDATA       15 /* server: insert user data; child: success/fail */
-#define REQ_PRINT_NL          16 /* child: print a newline */
+#define REQ_PRINTNEWLINE      16 /* child: print a newline */
+#define REQ_ALLDONE           17 /* child: break out of send_master() */
 
 /* child states, sent as an int to the master */
 #define STATE_INIT      0 /* initial state */