Merge branch 'refs/heads/t/clean_server_exit'
authorAndre Noll <maan@tuebingen.mpg.de>
Sun, 20 May 2018 09:26:26 +0000 (11:26 +0200)
committerAndre Noll <maan@tuebingen.mpg.de>
Sun, 20 May 2018 09:33:27 +0000 (11:33 +0200)
This series removes many memory leaks of para_server by refactoring
the shutdown and signal handling code. Most of the leaks happen only
at shutdown and are hence harmless. But it is still good to plug
the leaks because this puts more focus on real memory leaks in the
valgrind output.

The merge conflicted rather badly due to the changes introduced with
the crypt branch that was merged last week. The resolution has been
thoroughly tested, though.

* refs/heads/t/clean_server_exit: (32 commits)
  command.c: Document return value of handle_connect().
  user_list: Make list head static.
  afs: Allow database switching on sighup.
  afs: Free current mood or playlist on exit.
  afs: Free status items on exit.
  afs: Shutdown signals on exit.
  server: Free parse result also in afs.
  afs: Deplete user list at startup.
  server: Free audio file header on exit.
  sender: Deplete ACLs on exit.
  Remove some unused includes from {dccp,http}_send.c.
  server: Make argument of user_list_init() constant.
  server: Deplete user list on exit.
  server: Combine user_list_init() and populate().
  server: Move para_fgets() to user_list.c.
  server: Initialize user list at compile time.
  server: Rename functions related to user lists.
  server: Constify return value of lookup_user().
  server: Let stat command handler perform cleanup on signals.
  server: Have afs process close the current mood on exit().
  ...

15 files changed:
1  2 
NEWS.md
afs.c
afs.h
aft.c
command.c
dccp_send.c
fd.c
http_send.c
send.h
send_common.c
server.c
server.h
udp_send.c
user_list.c
vss.c

diff --cc NEWS.md
+++ b/NEWS.md
@@@ -1,15 -1,6 +1,23 @@@
  NEWS
  ====
  
 +-------------------------------------------
 +0.6.2 (to be accounced) "elastic diversity"
 +-------------------------------------------
 +
 +- para_gui no longer waits up to one second to update the screen when
 +  the geometry of the terminal changes.
 +- Minor documentation improvements.
 +- Improvements to the crypto subsystem.
++- The server subcommand "task" has been deprecated. It still works,
++  but prints nothing. It will be removed in the next major release.
++- Server log output is now serialized, avoiding issues with partial
++  lines.
++- It is now possible to switch to a different afs database by changing
++  the server configuration and sending SIGHUP to the server process.
++
++Download: [tarball](./releases/paraslash-git.tar.xz)
 +
  ----------------------------------------
  0.6.1 (2017-09-23) "segmented iteration"
  ----------------------------------------
diff --cc afs.c
Simple merge
diff --cc afs.h
Simple merge
diff --cc aft.c
Simple merge
diff --cc command.c
+++ b/command.c
@@@ -874,12 -900,14 +898,14 @@@ static int run_command(struct command_c
   * the function if the connection was not authenticated when the timeout
   * expires.
   *
 - * \sa alarm(2), \ref crypt.c, \ref crypt.h.
+  * \return Standard.
+  *
 + * \sa alarm(2), \ref openssl.c, \ref crypt.h.
   */
__noreturn void handle_connect(int fd)
int handle_connect(int fd)
  {
        int ret;
 -      unsigned char rand_buf[CHALLENGE_SIZE + 2 * SESSION_KEY_LEN];
 +      unsigned char rand_buf[APC_CHALLENGE_SIZE + 2 * SESSION_KEY_LEN];
        unsigned char challenge_hash[HASH_SIZE];
        char *command = NULL, *buf = para_malloc(HANDSHAKE_BUFSIZE) /* must be on the heap */;
        size_t numbytes;
diff --cc dccp_send.c
@@@ -215,32 -217,43 +218,39 @@@ static char *dccp_status(void
        return result;
  }
  
- /**
-  * The init function of the dccp sender.
-  *
-  * \param s pointer to the dccp sender struct.
-  *
-  * It initializes all function pointers of \a s and starts
-  * listening on the given port.
+ /*
+  * Initialize the client list and the access control list and listen on the
+  * dccp port.
   */
void dccp_send_init(struct sender *s)
static void dccp_send_init(void)
  {
-       s->status = dccp_status;
-       s->send = NULL;
-       s->pre_select = dccp_pre_select;
-       s->post_select = dccp_post_select;
-       s->shutdown_clients = dccp_shutdown_clients;
-       s->resolve_target = NULL;
-       s->help = generic_sender_help;
-       s->client_cmds[SENDER_on] = dccp_com_on;
-       s->client_cmds[SENDER_off] = dccp_com_off;
-       s->client_cmds[SENDER_deny] = dccp_com_deny;
-       s->client_cmds[SENDER_allow] = dccp_com_allow;
-       s->client_cmds[SENDER_add] = NULL;
-       s->client_cmds[SENDER_delete] = NULL;
 -      int ret;
--
        init_sender_status(dss, OPT_RESULT(DCCP_ACCESS),
                OPT_UINT32_VAL(DCCP_PORT), OPT_UINT32_VAL(DCCP_MAX_CLIENTS),
                OPT_GIVEN(DCCP_DEFAULT_DENY));
 -      ret = generic_com_on(dss, IPPROTO_DCCP);
 -      if (ret < 0)
 -              PARA_ERROR_LOG("%s\n", para_strerror(-ret));
 +      generic_com_on(dss, IPPROTO_DCCP);
  }
+ /**
+  * The DCCP sender.
+  *
+  * This sender offers congestion control not available in plain TCP. Most
+  * methods of the sender structure are implemented as simple wrappers for the
+  * generic functions defined in \ref send_common.c. Like UDP streams, DCCP
+  * streams are sent FEC-encoded.
+  */
+ const struct sender dccp_sender = {
+       .name = "dccp",
+       .init = dccp_send_init,
+       .shutdown = dccp_shutdown,
+       .pre_select = dccp_pre_select,
+       .post_select = dccp_post_select,
+       .shutdown_clients = dccp_shutdown_clients,
+       .client_cmds = {
+               [SENDER_on] = dccp_com_on,
+               [SENDER_off] = dccp_com_off,
+               [SENDER_deny] = dccp_com_deny,
+               [SENDER_allow] = dccp_com_allow,
+       },
+       .help = generic_sender_help,
+       .status = dccp_status,
+ };
diff --cc fd.c
Simple merge
diff --cc http_send.c
  #include "send.h"
  #include "sched.h"
  #include "vss.h"
 -#include "net.h"
 +#include "close_on_fork.h"
  #include "fd.h"
  #include "chunk_queue.h"
- #include "acl.h"
  
  /** Message sent to clients that do not send a valid get request. */
  #define HTTP_ERR_MSG "HTTP/1.0 400 Bad Request\n"
@@@ -236,34 -239,48 +241,44 @@@ static char *http_status(void
        return generic_sender_status(hss, "http");
  }
  
- /**
-  * The init function of the http sender.
-  *
-  * \param s Pointer to the http sender struct.
-  *
-  * It initializes all function pointers of \a s, the client list and the access
-  * control list. If the autostart option was given, the tcp port is opened.
+ /*
+  * Initialize the client list and the access control list, and optionally
+  * listen on the tcp port.
   */
void http_send_init(struct sender *s)
static void http_send_init(void)
  {
-       s->status = http_status;
-       s->send = http_send;
-       s->pre_select = http_pre_select;
-       s->post_select = http_post_select;
-       s->shutdown_clients = http_shutdown_clients;
-       s->resolve_target = NULL;
-       s->help = generic_sender_help;
-       s->client_cmds[SENDER_on] = http_com_on;
-       s->client_cmds[SENDER_off] = http_com_off;
-       s->client_cmds[SENDER_deny] = http_com_deny;
-       s->client_cmds[SENDER_allow] = http_com_allow;
-       s->client_cmds[SENDER_add] = NULL;
-       s->client_cmds[SENDER_delete] = NULL;
 -      int ret;
--
        init_sender_status(hss, OPT_RESULT(HTTP_ACCESS),
                OPT_UINT32_VAL(HTTP_PORT), OPT_UINT32_VAL(HTTP_MAX_CLIENTS),
                OPT_GIVEN(HTTP_DEFAULT_DENY));
        if (OPT_GIVEN(HTTP_NO_AUTOSTART))
                return;
 -      ret = generic_com_on(hss, IPPROTO_TCP);
 -      if (ret < 0)
 -              PARA_ERROR_LOG("%s\n", para_strerror(-ret));
 +      generic_com_on(hss, IPPROTO_TCP);
  }
+ /**
+  * The HTTP sender.
+  *
+  * This is the only sender which does not FEC-encode the stream. This is not
+  * necessary because HTTP sits on top of TCP, a reliable transport which
+  * retransmits lost packets automatically. The sender employs per-client queues
+  * which queue chunks of audio data if they can not be sent immediately because
+  * the write operation would block. Most methods of the sender are implemented
+  * as wrappers for the generic functions defined in \ref send_common.c.
+  */
+ const struct sender http_sender = {
+       .name = "http",
+       .init = http_send_init,
+       .shutdown = http_shutdown,
+       .pre_select = http_pre_select,
+       .post_select = http_post_select,
+       .send = http_send,
+       .shutdown_clients = http_shutdown_clients,
+       .client_cmds = {
+               [SENDER_on] = http_com_on,
+               [SENDER_off] = http_com_off,
+               [SENDER_deny] = http_com_deny,
+               [SENDER_allow] = http_com_allow,
+       },
+       .help = generic_sender_help,
+       .status = http_status,
+ };
diff --cc send.h
--- 1/send.h
--- 2/send.h
+++ b/send.h
@@@ -186,7 -190,8 +190,8 @@@ void generic_com_allow(struct sender_co
                struct sender_status *ss);
  void generic_com_deny(struct sender_command_data *scd,
                struct sender_status *ss);
 -int generic_com_on(struct sender_status *ss, unsigned protocol);
 +void generic_com_on(struct sender_status *ss, unsigned protocol);
+ void generic_acl_deplete(struct list_head *acl);
  void generic_com_off(struct sender_status *ss);
  char *generic_sender_help(void);
  struct sender_client *accept_sender_client(struct sender_status *ss, fd_set *rfds);
diff --cc send_common.c
Simple merge
diff --cc server.c
+++ b/server.c
@@@ -456,7 -505,7 +505,8 @@@ static int init_afs(int argc, char **ar
                int i;
  
                afs_pid = getpid();
 +              crypt_shutdown();
+               user_list_deplete();
                for (i = argc - 1; i >= 0; i--)
                        memset(argv[i], 0, strlen(argv[i]));
                i = argc - lls_num_inputs(cmdline_lpr) - 1;
@@@ -514,10 -563,12 +564,12 @@@ static void server_init(int argc, char 
        /* become daemon */
        if (OPT_GIVEN(DAEMON))
                daemon_pipe = daemonize(true /* parent waits for SIGTERM */);
 -      init_random_seed_or_die();
+       server_pid = getpid();
 +      crypt_init();
        daemon_log_welcome("server");
-       init_ipc_or_die(); /* init mmd struct and mmd->lock */
+       init_ipc_or_die(); /* init mmd struct, mmd and log mutex */
        daemon_set_start_time();
+       daemon_set_hooks(pre_log_hook, post_log_hook);
        PARA_NOTICE_LOG("initializing audio format handlers\n");
        afh_init();
  
@@@ -607,15 -675,32 +676,32 @@@ int main(int argc, char *argv[]
        sched.default_timeout.tv_sec = 1;
        sched.select_function = server_select;
  
-       server_init(argc, argv);
+       server_init(argc, argv, sct);
        mutex_lock(mmd_mutex);
        ret = schedule(&sched);
+       /*
+        * We hold the mmd lock: it was re-acquired in server_select()
+        * after the select call.
+        */
+       mutex_unlock(mmd_mutex);
        sched_shutdown(&sched);
-       lls_free_parse_result(server_lpr, CMD_PTR);
-       if (server_lpr != cmdline_lpr)
-               lls_free_parse_result(cmdline_lpr, CMD_PTR);
-       if (ret < 0)
-               PARA_EMERG_LOG("%s\n", para_strerror(-ret));
 +      crypt_shutdown();
 -              shm_detach(mmd);
+       signal_shutdown(signal_task);
+       if (!process_is_command_handler()) { /* parent (server) */
+               mutex_destroy(mmd_mutex);
+               daemon_set_hooks(NULL, NULL); /* only one process remaining */
+               mutex_destroy(log_mutex);
+               deplete_close_on_fork_list();
+               if (ret < 0)
+                       PARA_EMERG_LOG("%s\n", para_strerror(-ret));
+       } else {
+               alarm(ALARM_TIMEOUT);
+               close_listed_fds();
+               ret = handle_connect(sct->child_fd);
+       }
+       vss_shutdown();
+       shm_detach(mmd);
+       user_list_deplete();
+       free_lpr();
        exit(ret < 0? EXIT_FAILURE : EXIT_SUCCESS);
  }
diff --cc server.h
Simple merge
diff --cc udp_send.c
Simple merge
diff --cc user_list.c
  #include "list.h"
  #include "user_list.h"
  
- static struct list_head user_list;
+ static INITIALIZED_LIST_HEAD(user_list);
  
  /*
-  * Fill the list of users known to para_server.
+  * Wrapper for fgets(3).
   *
-  * Populates a linked list of all users in \a user_list_file.  Returns on
-  * success, calls exit() on errors.
+  * Unlike fgets(3), an integer value is returned. On success, this function
+  * returns 1. On errors, -E_FGETS is returned. A zero return value indicates an
+  * end of file condition.
   */
- static void populate_user_list(char *user_list_file)
+ static int xfgets(char *line, int size, FILE *f)
+ {
+ again:
+       if (fgets(line, size, f))
+               return 1;
+       if (feof(f))
+               return 0;
+       if (!ferror(f))
+               return -E_FGETS;
+       if (errno != EINTR) {
+               PARA_ERROR_LOG("%s\n", strerror(errno));
+               return -E_FGETS;
+       }
+       clearerr(f);
+       goto again;
+ }
+ /**
+  * Remove all entries from the user list.
+  *
+  * This is called on shutdown and when the user list is reloaded because the
+  * server received SIGHUP.
+  */
+ void user_list_deplete(void)
+ {
+       struct user *u, *tmpu;
+       list_for_each_entry_safe(u, tmpu, &user_list, node) {
+               list_del(&u->node);
+               free(u->name);
 -              free_public_key(u->pubkey);
++              apc_free_pubkey(u->pubkey);
+               free(u);
+       }
+ }
+ /**
+  * Initialize the list of users allowed to connect to para_server.
+  *
+  * \param user_list_file The file containing access information.
+  *
+  * If this function is called for the second time, the contents of the
+  * previous call are discarded, i.e. the user list is reloaded.
+  *
+  * This function either succeeds or calls exit(3).
+  */
+ void user_list_init(const char *user_list_file)
  {
        int ret = -E_USERLIST;
        FILE *file_ptr = fopen(user_list_file, "r");
diff --cc vss.c
Simple merge