Merge lp:~jamesodhunt/upstart/bug-1157713-inotify-test-failures into lp:upstart

[James Hunt]

* init/Makefile.am: Build wrap_inotify library and run test_conf
  via test_conf_preload.sh.
* init/tests/test_conf.c: Communicate with wrap_inotify library by
  setting INOTIFY_DISABLE to reliably disable inotify rather than trying
  to exhaust inotify instances (LP: #1157713).
* init/tests/test_conf_preload.sh.in: Script to run test_conf within
  LD_PRELOAD environment.
* init/tests/wrap_inotify.c: Wrapper library that provides the inotify
  API and allows test_conf to believe inotify is disabled by
  conditionally failing all inotify calls, depending on whether
  INOTIFY_DISABLE is set.

[lp:~jamesodhunt/upstart/bug-1157713-inotify-test-failures]

1474. By James Hunt on 2013-04-29

* init/tests/wrap_inotify.c: Use RTLD_NEXT to avoid having to call dlopen.

[Dimitri John Ledkov (xnox)]

lgtm, resolve changelog conflict and merge.

review: Approve

[lp:~jamesodhunt/upstart/bug-1157713-inotify-test-failures]

1475. By James Hunt on 2013-04-30

Sync with upstream.

=== modified file 'ChangeLog'
--- ChangeLog	2013-04-22 10:30:09 +0000
+++ ChangeLog	2013-04-30 23:28:25 +0000
@@ -2,6 +2,20 @@
 
 	* Typo and doc changes.
 
[phone]  James Hunt  <[e-mail]>
+
+	* init/Makefile.am: Build wrap_inotify library and run test_conf
+	  via test_conf_preload.sh.
+	* init/tests/test_conf.c: Communicate with wrap_inotify library by
+	  setting INOTIFY_DISABLE to reliably disable inotify rather than trying
+	  to exhaust inotify instances (LP: #1157713).
+	* init/tests/test_conf_preload.sh.in: Script to run test_conf within
+	  LD_PRELOAD environment.
+	* init/tests/wrap_inotify.c: Wrapper library that provides the inotify
+	  API and allows test_conf to believe inotify is disabled by
+	  conditionally failing all inotify calls, depending on whether
+	  INOTIFY_DISABLE is set.
+
 2013-04-02  James Hunt  <[e-mail]>
 
 	* extra/man/file-event.7: Correct EVENT values in examples.

=== modified file 'init/Makefile.am'
--- init/Makefile.am	2013-02-14 17:24:05 +0000
+++ init/Makefile.am	2013-04-30 23:28:25 +0000
@@ -136,8 +136,6 @@
 TEST_DATA_FILES = \
 	$(TEST_DATA_DIR)/upstart-1.6.json
 
-EXTRA_DIST = init.supp $(TEST_DATA_FILES)
-
 test_util_SOURCES = \
 	tests/test_util.c tests/test_util.h
 
@@ -155,14 +153,23 @@
 	test_blocked \
 	test_parse_job \
 	test_parse_conf \
-	test_conf \
 	test_conf_static \
 	test_xdg \
 	test_control
 
-check_PROGRAMS = $(TESTS)
-
-tests: $(BUILT_SOURCES) $(check_PROGRAMS)
+EXTRA_DIST = init.supp $(TEST_DATA_FILES) tests/test_conf_preload.sh.in
+
+test_conf_preload.sh: tests/test_conf_preload.sh.in Makefile test_conf
+	sed -e 's|[@]abs_builddir[@]|$(abs_builddir)|g' \
+	    -e 's|[@]inotify_preload_library[@]|$(check_LTLIBRARIES)|g' \
+	    $< > $@
+	chmod +x $@
+
+check_PROGRAMS = $(TESTS) test_conf
+check_SCRIPTS = test_conf_preload.sh
+CLEANFILES += $(check_SCRIPTS)
+
+tests: $(BUILT_SOURCES) $(check_PROGRAMS) $(check_LTLIBRARIES)
 
 test_system_SOURCES = tests/test_system.c
 test_system_LDADD = \
@@ -328,7 +335,12 @@
 	$(JSON_LIBS) \
 	-lrt
 
-test_conf_SOURCES = tests/test_conf.c $(test_util_SOURCES)
+check_LTLIBRARIES = tests/libwrap_inotify.la
+tests_libwrap_inotify_la_SOURCES = tests/wrap_inotify.c
+tests_libwrap_inotify_la_LIBADD = -lrt -ldl $(AM_LIBADD)
+tests_libwrap_inotify_la_LDFLAGS = avoid-version -module -shared -export-dynamic -rpath /nowhere -ldl
+
+test_conf_SOURCES = tests/test_conf.c $(test_util_SOURCES) $(check_LTLIBRARIES)
 test_conf_LDADD = \
 	system.o environ.o process.o \
 	job_class.o job_process.o job.o event.o event_operator.o blocked.o \

=== modified file 'init/tests/test_conf.c'
--- init/tests/test_conf.c	2013-01-25 09:01:00 +0000
+++ init/tests/test_conf.c	2013-04-30 23:28:25 +0000
@@ -126,7 +126,7 @@
 	JobClass   *job, *old_job;
 	Job        *instance;
 	FILE       *f;
-	int         ret, fd[4096], i = 0, nfds;
+	int         ret, fd, nfds;
 	char        dirname[PATH_MAX];
 	char        tmpname[PATH_MAX], filename[PATH_MAX];
 	fd_set      readfds, writefds, exceptfds;
@@ -171,11 +171,11 @@
 	fclose (f);
 
 	/* Make sure that we have inotify before performing some tests... */
-	if ((fd[0] = inotify_init ()) < 0) {
+	if ((fd = inotify_init ()) < 0) {
 		printf ("SKIP: inotify not available\n");
 		goto no_inotify;
 	}
-	close (fd[0]);
+	close (fd);
 
 
 	/* Check that we can load a job directory source for the first time.
@@ -1083,15 +1083,10 @@
 	unlink (filename);
 	rmdir (dirname);
 
-
-	/* Consume all available inotify instances so that the following
-	 * tests run without inotify.
-	 */
-	for (i = 0; i < 4096; i++)
-		if ((fd[i] = inotify_init ()) < 0)
-			break;
 no_inotify:
 
+	/* Disable inotify for the following tests */
+	assert0 (putenv ("INOTIFY_DISABLE=1"));
 
 	TEST_FILENAME (dirname);
 	mkdir (dirname, 0755);
@@ -1614,13 +1609,8 @@
 
 	nih_log_set_priority (NIH_LOG_MESSAGE);
 
-	/* Release consumed instances */
-	for (i = 0; i < 4096; i++) {
-		if (fd[i] < 0)
-			break;
-
-		close (fd[i]);
-	}
+	/* Re-enable inotify */
+	assert0 (unsetenv ("INOTIFY_DISABLE"));
 }
 
 
@@ -1630,7 +1620,7 @@
 	ConfSource *source;
 	ConfFile   *file, *old_file;
 	FILE       *f;
-	int         ret, fd[4096], i = 0, nfds;
+	int         ret, fd, nfds;
 	char        dirname[PATH_MAX];
 	char        filename[PATH_MAX];
 	fd_set      readfds, writefds, exceptfds;
@@ -1671,11 +1661,11 @@
 	fclose (f);
 
 	/* Make sure that we have inotify before performing some tests... */
-	if ((fd[0] = inotify_init ()) < 0) {
+	if ((fd = inotify_init ()) < 0) {
 		printf ("SKIP: inotify not available\n");
 		goto no_inotify;
 	}
-	close (fd[0]);
+	close (fd);
 
 
 	/* Check that we can load a conf directory source for the first time.
@@ -2047,15 +2037,10 @@
 
 	nih_free (source);
 
-
-	/* Consume all available inotify instances so that the following
-	 * tests run without inotify.
-	 */
-	for (i = 0; i < 4096; i++)
-		if ((fd[i] = inotify_init ()) < 0)
-			break;
 no_inotify:
 
+	/* Disable inotify for the following tests */
+	assert0 (putenv ("INOTIFY_DISABLE=1"));
 
 	TEST_FILENAME (dirname);
 	mkdir (dirname, 0755);
@@ -2409,13 +2394,8 @@
 
 	nih_log_set_priority (NIH_LOG_MESSAGE);
 
-	/* Release consumed instances */
-	for (i = 0; i < 4096; i++) {
-		if (fd[i] < 0)
-			break;
-
-		close (fd[i]);
-	}
+	/* Re-enable inotify */
+	assert0 (unsetenv ("INOTIFY_DISABLE"));
 }
 
 void
@@ -2424,7 +2404,7 @@
 	ConfSource *source;
 	ConfFile   *file;
 	FILE       *f;
-	int         ret, fd[4096], i = 0;
+	int         ret, fd;
 	char        dirname[PATH_MAX];
 	char        filename[PATH_MAX], override[PATH_MAX], override2[PATH_MAX];
 	char       *dir;
@@ -2438,11 +2418,11 @@
 	TEST_GROUP ("override files");
 
 	/* Make sure that we have inotify before performing some tests... */
-	if ((fd[0] = inotify_init ()) < 0) {
+	if ((fd = inotify_init ()) < 0) {
 		printf ("SKIP: inotify not available\n");
 		goto no_inotify;
 	}
-	close (fd[0]);
+	close (fd);
 
 
 	/* Explicit test of behaviour prior to introduction of override files.
@@ -3665,14 +3645,11 @@
 	unlink (override);
 	TEST_EQ (rmdir (dirname), 0);
 
-	/* Consume all available inotify instances so that the following
-	 * tests run without inotify.
-	 */
-	for (i = 0; i < 4096; i++)
-		if ((fd[i] = inotify_init ()) < 0)
-			break;
-
 no_inotify:
+
+	/* Disable inotify for the following tests */
+	assert0 (putenv ("INOTIFY_DISABLE=1"));
+
 	/* If you don't have inotify, any override file must exist
 	 * before the system boots.
 	 */ 
@@ -3729,13 +3706,8 @@
 
 	nih_log_set_priority (NIH_LOG_MESSAGE);
 
-	/* Release consumed instances */
-	for (i = 0; i < 4096; i++) {
-		if (fd[i] < 0)
-			break;
-
-		close (fd[i]);
-	}
+	/* Re-enable inotify */
+	assert0 (unsetenv ("INOTIFY_DISABLE"));
 }
 
 void
@@ -3744,7 +3716,7 @@
 	ConfSource *source;
 	ConfFile   *file, *old_file;
 	FILE       *f;
-	int         ret, fd[4096], i = 0, nfds;
+	int         ret, fd, nfds;
 	char        dirname[PATH_MAX];
 	char        tmpname[PATH_MAX], filename[PATH_MAX];
 	fd_set      readfds, writefds, exceptfds;
@@ -3773,11 +3745,11 @@
 	fclose (f);
 
 	/* Make sure that we have inotify before performing some tests... */
-	if ((fd[0] = inotify_init ()) < 0) {
+	if ((fd = inotify_init ()) < 0) {
 		printf ("SKIP: inotify not available\n");
 		goto no_inotify;
 	}
-	close (fd[0]);
+	close (fd);
 
 
 	/* Check that we can load a file source for the first time.  An
@@ -4289,14 +4261,11 @@
 	TEST_HASH_EMPTY (source->files);
 
 	nih_free (source);
-	/* Consume all available inotify instances so that the following
-	 * tests run without inotify.
-	 */
-	for (i = 0; i < 4096; i++)
-		if ((fd[i] = inotify_init ()) < 0)
-			break;
+
 no_inotify:
 
+	/* Disable inotify for the following tests */
+	assert0 (putenv ("INOTIFY_DISABLE=1"));
 
 	TEST_FILENAME (dirname);
 	mkdir (dirname, 0755);
@@ -4524,13 +4493,8 @@
 
 	nih_log_set_priority (NIH_LOG_MESSAGE);
 
-	/* Release consumed instances */
-	for (i = 0; i < 4096; i++) {
-		if (fd[i] < 0)
-			break;
-
-		close (fd[i]);
-	}
+	/* Re-enable inotify */
+	assert0 (unsetenv ("INOTIFY_DISABLE"));
 }
 
 

=== added file 'init/tests/test_conf_preload.sh.in'
--- init/tests/test_conf_preload.sh.in	1970-01-01 00:00:00 +0000
+++ init/tests/test_conf_preload.sh.in	2013-04-30 23:28:25 +0000
@@ -0,0 +1,64 @@
+#!/bin/sh -e
+#---------------------------------------------------------------------
+# Script to run a test in a modified environment where inotify can be
+# disabled.
+#---------------------------------------------------------------------
+# Copyright (C) 2013 Canonical Ltd.
+#
+# Author: James Hunt <[e-mail]>
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, version 3 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+#---------------------------------------------------------------------
+
+die()
+{
+    msg="$*"
+    echo "ERROR: $msg" >&2
+    exit 1
+}
+
+# Name of test to run in modified environment
+test_name=test_conf
+
+build_dir="@abs_builddir@"
+test_path="$build_dir/$test_name"
+
+preload_library="$build_dir/@inotify_preload_library@"
+
+for file in "$test_path" "$preload_library"
+do
+    [ -e "$file" ] || die "file $file not found"
+done
+
+# Install the preload library to a temporary location since we need the
+# full path to the shared library for LD_PRELOAD.
+install_dir=$(mktemp -d --tmpdir="$builddir")
+libtool --mode=install install \
+    "$preload_library" "$install_dir" >/dev/null 2>&1
+
+installed_lib=$(basename "$preload_library")
+installed_so="$install_dir/$(echo "$installed_lib"|cut -d\. -f1).so"
+
+[ -f "$installed_so" ] || die "cannot find $installed_so"
+
+echo
+echo "INFO: Running test $test_name in LD_PRELOAD environment which allows inotify to be disabled/enabled"
+echo
+
+# Run test in modified environment
+LD_PRELOAD=$installed_so $test_path
+
+# clean up
+libtool --mode=uninstall \
+    rm "$install_dir/$installed_lib" >/dev/null 2>&1

=== added file 'init/tests/wrap_inotify.c'
--- init/tests/wrap_inotify.c	1970-01-01 00:00:00 +0000
+++ init/tests/wrap_inotify.c	2013-04-30 23:28:25 +0000
@@ -0,0 +1,177 @@
+/* upstart
+ *
+ * wrap_inotify.c - library to subvert inotify calls.
+ *
+ * Copyright © Â 2013 Canonical Ltd.
+ * Author: James Hunt <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+/**
+ * = Description =
+ *
+ * The test_conf upstart test requires certain test scenarios to run in
+ * an environment where inotify(7) is not available/functional to force the
+ * underlying NIH library to perform a manual filesystem tree traversal.
+ *
+ * Since inotify limits are _per user_ and not _per process_, it is not
+ * possible to disable inotify on a system reliabliy for the duration
+ * of a test run since the test is at the mercy of other processes that
+ * are making inotify calls too.
+ * 
+ * The only reliable method therefore is to "fake" the inotify calls
+ * using this library.
+ *
+ * To use this library:
+ *
+ * 1) Have the test code set the environment variable "INOTIFY_DISABLE" to
+ *    any value to disable inotify, and unset the variable to leave it
+ *    enabled.
+ *
+ * 2) Run the test code using LD_PRELOAD to force the dynamic
+ *    link-loader to use these inotify definitions rather than those
+ *    provided by libc:
+ *
+ *        (LD_PRELOAD=/path/to/this/libary.so test_code)
+ *
+ * To convince yourself this library is being used, set "INOTIFY_DEBUG"
+ * to any value for some stdout debug messages.
+ **/
+
+/* to ensure we get RTLD_NEXT */
+#define _GNU_SOURCE
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <assert.h>
+#include <sys/inotify.h>
+#include <dlfcn.h>
+
+/**
+ * Determine if inotify should be disabled.
+ **/
+#define disable_inotify() \
+    (getenv ("INOTIFY_DISABLE"))
+
+/**
+ * Determine if inotify debug should be displayed to stdout.
+ **/
+#define debug_inotify() \
+    (getenv ("INOTIFY_DEBUG"))
+
+/**
+ * debug_msg:
+ *
+ * If debug is enabled, display a message to stdout stating if inotify
+ * is enabled along with details of the called function.
+ **/
+#define debug_msg() \
+	do { \
+		if (debug_inotify ()) { \
+			printf ("DEBUG:%s:%d: inotify %s\n", \
+					__func__, __LINE__, \
+					disable_inotify () \
+					? "disabled" : "enabled"); \
+			fflush (NULL); \
+		} \
+	} while (0)
+
+int __wrap_inotify_init (void)
+	__attribute ((warn_unused_result, no_instrument_function));
+
+int __wrap_inotify_add_watch (int fd, const char *pathname, uint32_t mask)
+	__attribute ((warn_unused_result, no_instrument_function));
+
+int __wrap_inotify_rm_watch (int fd, int wd)
+	__attribute ((warn_unused_result, no_instrument_function));
+
+int (*real_inotify_init_addr) (void);
+int (*real_inotify_add_watch_addr) (int fd, const char *pathname, uint32_t mask);
+int (*real_inotify_rm_watch_addr) (int fd, int wd);
+
+int
+__wrap_inotify_init (void)
+{
+	if (disable_inotify ()) {
+		/* simulate reaching inotify instances user limit */
+		errno = EMFILE;
+		return -1;
+	}
+
+	*(void **)(&real_inotify_init_addr) = dlsym (RTLD_NEXT, "inotify_init");
+
+	assert (! dlerror ()); 
+	assert (real_inotify_init_addr);
+
+	return real_inotify_init_addr ();
+}
+
+int
+__wrap_inotify_add_watch (int fd, const char *pathname, uint32_t mask)
+{
+	if (disable_inotify ()) {
+		/* simulate reaching inotify watches user limit */
+		errno = ENOSPC;
+		return -1;
+	}
+
+	*(void **)(&real_inotify_add_watch_addr) = dlsym (RTLD_NEXT, "inotify_add_watch");
+
+	assert (! dlerror ()); 
+	assert (real_inotify_add_watch_addr);
+
+	return real_inotify_add_watch_addr (fd, pathname, mask);
+}
+
+int
+__wrap_inotify_rm_watch (int fd, int wd)
+{
+	if (disable_inotify ()) {
+		; /* not meaningful, so just pass through */
+	}
+
+	*(void **)(&real_inotify_rm_watch_addr) = dlsym (RTLD_NEXT, "inotify_rm_watch");
+
+	assert (! dlerror ()); 
+	assert (real_inotify_rm_watch_addr);
+
+	return real_inotify_rm_watch_addr (fd, wd);
+}
+
+int
+inotify_init (void)
+{
+	debug_msg ();
+
+	return __wrap_inotify_init ();
+}
+
+int
+inotify_add_watch (int fd, const char *pathname, uint32_t mask)
+{
+	debug_msg ();
+
+	return __wrap_inotify_add_watch (fd, pathname, mask);
+
+}
+
+int
+inotify_rm_watch (int fd, int wd)
+{
+	debug_msg ();
+
+	return __wrap_inotify_rm_watch (fd, wd);
+}