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

@@ -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 */

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>

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>
 

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")

test/box/tarantool_bug750658.cfg

+#
+# 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"
+

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)

third_party/CMakeLists.txt

-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

third_party/daemon.c

-/*    $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);
-}