From 58c6dd3643634511b251bb43e2ba54cc1ac46431 Mon Sep 17 00:00:00 2001 From: Konstantin Osipov <kostja.osipov@gmail.com> Date: Thu, 13 Oct 2011 17:59:22 +0400 Subject: [PATCH] A fix and a test case for Bug#750658 A fix and a test case for https://bugs.launchpad.net/tarantool/+bug/750658 --background neither closes nor redirects stdin/stdout/stderr --background option didn't work properly, since stdin/stdout/ stderr streams were left open and pointing to a terminal. This lead to a hang when tarantool was started from a shell script or over ssh. If --background option is given, we need to fork, try to create a pid file, close stdin/stdout/stderr and then initialize the logging subsystem. --- core/tarantool.m | 51 ++++++++++++++++--- doc/user/preface.xml | 2 +- test/box/args.result | 9 ++++ test/box/args.test | 10 ++++ test/box/tarantool_bug750658.cfg | 40 +++++++++++++++ test/lib/server.py | 6 +-- third_party/CMakeLists.txt | 2 +- third_party/daemon.c | 87 -------------------------------- 8 files changed, 108 insertions(+), 99 deletions(-) create mode 100644 test/box/tarantool_bug750658.cfg delete mode 100644 third_party/daemon.c diff --git a/core/tarantool.m b/core/tarantool.m index 2ec83aaa29..46a513a8c2 100644 --- a/core/tarantool.m +++ b/core/tarantool.m @@ -73,8 +73,6 @@ static ev_signal *sigs = NULL; bool init_storage, booting = true; -extern int daemonize(int nochdir, int noclose); - static i32 load_cfg(struct tarantool_cfg *conf, i32 check_rdonly) { @@ -288,6 +286,9 @@ create_pid(void) char buf[16] = { 0 }; pid_t pid; + if (cfg.pid_file == NULL) + return; + f = fopen(cfg.pid_file, "a+"); if (f == NULL) panic_syserror("can't open pid file"); @@ -315,6 +316,36 @@ create_pid(void) fclose(f); } +/** Run in the background. + */ +static void background() +{ + switch (fork()) { + case -1: + goto error; + case 0: /* child */ + break; + default: /* parent */ + exit(EXIT_SUCCESS); + } + + if (setsid() == -1) + goto error; + + /* + * Prints to stdout on failure, so got to be done before we + * close it. + */ + create_pid(); + + close(STDIN_FILENO); + close(STDOUT_FILENO); + close(STDERR_FILENO); + +error: + exit(EXIT_FAILURE); +} + void tarantool_free(void) { @@ -542,11 +573,18 @@ main(int argc, char **argv) exit(EXIT_SUCCESS); } - if (gopt(opt, 'B')) - daemonize(1, 1); - - if (cfg.pid_file != NULL) + if (gopt(opt, 'B')) { + if (cfg.logger == NULL) { + say_crit("--background requires 'logger' configuration option to be set"); + exit(EXIT_FAILURE); + } + background(); + } + else { create_pid(); + } + + say_logger_init(cfg.logger_nonblock); /* init process title */ if (cfg.custom_proc_title == NULL) { @@ -557,7 +595,6 @@ main(int argc, char **argv) strcat(custom_proc_title, cfg.custom_proc_title); } - say_logger_init(cfg.logger_nonblock); booting = false; /* main core cleanup routine */ diff --git a/doc/user/preface.xml b/doc/user/preface.xml index 09b6362682..16c12ddbf6 100644 --- a/doc/user/preface.xml +++ b/doc/user/preface.xml @@ -101,7 +101,7 @@ <para> UNIX shell command input is prefixed with '$ ' and is formatted using a fixed-width font: - <programlisting><prompt>$ </prompt>tarantool_box <option>--daemonize</option> + <programlisting><prompt>$ </prompt>tarantool_box <option>--background</option> </programlisting> </para> <para> diff --git a/test/box/args.result b/test/box/args.result index a38e766368..7bf5ec89e5 100644 --- a/test/box/args.result +++ b/test/box/args.result @@ -64,6 +64,15 @@ tarantool_box: --daemonize: unknown option tarantool_box --background tarantool_box: the daemon is already running +# +# Check that --background doesn't work if there is no logger +# This is a test case for +# https:tarantool/+bug/750658 +# "--background neither closes nor redirects stdin/stdout/stderr" +# +tarantool_box --config=tarantool_bug750658.cfg --background +tarantool_box: --background requires 'logger' configuration option to be set + tarantool_box --version Tarantool/Box 1.minor.patch-<rev>-<commit> diff --git a/test/box/args.test b/test/box/args.test index a60b30a345..f51766a25a 100644 --- a/test/box/args.test +++ b/test/box/args.test @@ -15,6 +15,16 @@ server.test_option("-c") server.test_option("--config tarantool.cfg") server.test_option("--daemonize") server.test_option("--background") +print """# +# Check that --background doesn't work if there is no logger +# This is a test case for +# https://bugs.launchpad.net/tarantool/+bug/750658 +# "--background neither closes nor redirects stdin/stdout/stderr" +#""" +cfg = os.path.join(vardir, "tarantool_bug750658.cfg") +os.symlink(os.path.abspath("box/tarantool_bug750658.cfg"), cfg) +server.test_option("--config=tarantool_bug750658.cfg --background") +os.unlink(cfg) sys.stdout.pop_filter() sys.stdout.push_filter("(\d)\.\d\.\d(-\d+-\w+)?", "\\1.minor.patch-<rev>-<commit>") server.test_option("--version") diff --git a/test/box/tarantool_bug750658.cfg b/test/box/tarantool_bug750658.cfg new file mode 100644 index 0000000000..13a2c47be6 --- /dev/null +++ b/test/box/tarantool_bug750658.cfg @@ -0,0 +1,40 @@ +# +# Limit of memory used to store tuples to 100MB +# (0.1 GB) +# This effectively limits the memory, used by +# Tarantool. However, index and connection memory +# is stored outside the slab allocator, hence +# the effective memory usage can be higher (sometimes +# twice as high). +# +slab_alloc_arena = 0.1 + +# +# Store the pid in this file. Relative to +# startup dir. +# +pid_file = "box.pid" + +# +# Read only and read-write port. +primary_port = 33013 +# Read-only port. +secondary_port = 33014 +# +# The port for administrative commands. +# +admin_port = 33015 +# +# Each write ahead log contains this many rows. +# When the limit is reached, Tarantool closes +# the WAL and starts a new one. +rows_per_wal = 50 + +# Define a simple space with 1 HASH-based +# primary key. +space[0].enabled = 1 +space[0].index[0].type = "HASH" +space[0].index[0].unique = 1 +space[0].index[0].key_field[0].fieldno = 0 +space[0].index[0].key_field[0].type = "NUM" + diff --git a/test/lib/server.py b/test/lib/server.py index cd9f0fe7b2..bd63f44f9d 100644 --- a/test/lib/server.py +++ b/test/lib/server.py @@ -224,7 +224,7 @@ class Server(object): self.process.expect(pexpect.EOF) self.process.close() - self.wait_until_stoped() + self.wait_until_stopped() # clean-up processs flags self.is_started = False self.process = None @@ -304,8 +304,8 @@ class Server(object): except socket.error as e: time.sleep(0.001) - def wait_until_stoped(self): - """Wait until the server is stoped and close sockets""" + def wait_until_stopped(self): + """Wait until the server is stoped and has closed sockets""" while self.read_pidfile() != -1: time.sleep(0.001) diff --git a/third_party/CMakeLists.txt b/third_party/CMakeLists.txt index a538857849..a6715612cc 100644 --- a/third_party/CMakeLists.txt +++ b/third_party/CMakeLists.txt @@ -1,4 +1,4 @@ -add_library (misc STATIC crc32.c daemon.c proctitle.c qsort_arg.c) +add_library (misc STATIC crc32.c proctitle.c qsort_arg.c) if (TARGET_OS_FREEBSD) set_source_files_properties(proctitle.c PROPERTIES diff --git a/third_party/daemon.c b/third_party/daemon.c deleted file mode 100644 index 54216f318a..0000000000 --- a/third_party/daemon.c +++ /dev/null @@ -1,87 +0,0 @@ -/* $Header: /cvsroot/wikipedia/willow/src/bin/willow/daemon.c,v 1.1 2005/05/02 19:15:21 kateturner Exp $ */ -/* $NetBSD: daemon.c,v 1.9 2003/08/07 16:42:46 agc Exp $ */ -/*- - * Copyright (c) 1990, 1993 - * The Regents of the University of California. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. Neither the name of the University nor the names of its contributors - * may be used to endorse or promote products derived from this software - * without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#if defined __SUNPRO_C || defined __DECC || defined __HP_cc -# pragma ident "@(#)$Header: /cvsroot/wikipedia/willow/src/bin/willow/daemon.c,v 1.1 2005/05/02 19:15:21 kateturner Exp $" -# pragma ident "$NetBSD: daemon.c,v 1.9 2003/08/07 16:42:46 agc Exp $" -#endif - -#include <fcntl.h> -#include <stdio.h> -#include <stdlib.h> -#include <unistd.h> - -int daemonize(int nochdir, int noclose) -{ - int fd; - - switch (fork()) { - case -1: - return (-1); - case 0: - break; - default: - _exit(EXIT_SUCCESS); - } - - if (setsid() == -1) - return (-1); - - if (nochdir == 0) { - if(chdir("/") != 0) { - perror("chdir"); - return (-1); - } - } - - if (noclose == 0 && (fd = open("/dev/null", O_RDWR, 0)) != -1) { - if(dup2(fd, STDIN_FILENO) < 0) { - perror("dup2 stdin"); - return (-1); - } - if(dup2(fd, STDOUT_FILENO) < 0) { - perror("dup2 stdout"); - return (-1); - } - if(dup2(fd, STDERR_FILENO) < 0) { - perror("dup2 stderr"); - return (-1); - } - - if (fd > STDERR_FILENO) { - if(close(fd) < 0) { - perror("close"); - return (-1); - } - } - } - return (0); -} -- GitLab