Improve server start/stop logic

Unlink server pidfile on orderly exit

Only call serverCleanUp() in server process, not in "start" command
(fixes race condition)
This commit is contained in:
Andrew Bettison 2014-06-11 14:54:31 +09:30
parent a1c42eb378
commit d46939f81e
2 changed files with 34 additions and 21 deletions

View File

@ -1024,12 +1024,11 @@ int app_server_stop(const struct cli_parsed *parsed, struct cli_context *context
}
++tries;
if (kill(pid, SIGHUP) == -1) {
// ESRCH means process is gone, possibly we are racing with another stop, or servald just
// died voluntarily.
if (errno == ESRCH) {
serverCleanUp();
// ESRCH means process is gone, possibly we are racing with another stop, or servald just died
// voluntarily. We DO NOT call serverCleanUp() in this case (once used to!) because that
// would race with a starting server process.
if (errno == ESRCH)
break;
}
WHY_perror("kill");
WHYF("Error sending SIGHUP to Servald pid=%d (pidfile %s)", pid, server_pidfile_path());
return 252;

View File

@ -18,6 +18,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
#include <assert.h>
#include <dirent.h>
#include <signal.h>
#include <unistd.h>
@ -43,10 +44,11 @@ int serverMode = 0;
keyring_file *keyring=NULL;
static char pidfile_path[256];
static int server_getpid = 0;
void signal_handler(int signal);
static int server_write_pid();
static int server_unlink_pid();
static void signal_handler(int signal);
static void serverCleanUp();
/** Return the PID of the currently running server process, return 0 if there is none.
*/
@ -156,17 +158,21 @@ int server()
// log message used by tests to wait for the server to start
INFO("Server initialised, entering main loop");
/* Check for activitiy and respond to it */
while((serverMode==1) && fd_poll());
while (serverMode == 1 && fd_poll())
;
serverCleanUp();
/* It is safe to unlink the pidfile here without checking whether it actually contains our own
* PID, because server_shutdown_check() will have been executed very recently (in fd_poll()), so
* if the code reaches here, the check has been done recently.
*/
server_unlink_pid();
RETURN(0);
OUT();
}
int server_write_pid()
static int server_write_pid()
{
/* Record PID to advertise that the server is now running */
const char *ppath = server_pidfile_path();
@ -181,6 +187,17 @@ int server_write_pid()
return 0;
}
static int server_unlink_pid()
{
/* Remove PID file to indicate that the server is no longer running */
const char *ppath = server_pidfile_path();
if (ppath == NULL)
return -1;
if (unlink(ppath) == -1)
WHYF_perror("unlink(%s)", alloca_str_toprint(ppath));
return 0;
}
static int get_proc_path(const char *path, char *buf, size_t bufsiz)
{
if (!formf_serval_run_path(buf, bufsiz, PROC_SUBDIR "/%s", path))
@ -422,16 +439,13 @@ static void clean_proc()
}
}
void serverCleanUp()
static void serverCleanUp()
{
if (serverMode){
rhizome_close_db();
dna_helper_shutdown();
overlay_interface_close_all();
}
assert(serverMode);
rhizome_close_db();
dna_helper_shutdown();
overlay_interface_close_all();
overlay_mdp_clean_socket_files();
clean_proc();
}