Merge lp:~jamesodhunt/libnih/libnih-inotify-overflow-fix-for-777093 into lp:libnih

[James Hunt]

  Special case code to handle inotify event queue overflow (LP:#777093).

  * nih/watch.c: nih_watch_reader(): Handle situation where kernel
    is generating events faster than can be consumed by simply
    ignoring them. Failure to do so results in an assertion failure in
    nih_watch_handle_by_wd() since the inotify_event object will
    contain an invalid watch descriptor (-1).
  * nih/tests/test_watch.c:
    - Added PROC_MAX_INOTIFY define.
    - test_reader(): Added test to check for inotify event queue overflow
      handling.

[Scott James Remnant (scott)]

NAK.

This code is still not "handling" the problem, it's just ignoring it.

Take Upstart for example; it uses inotify to watch its configuration directory. A queue overflow could mean someone just untarred a giant /etc/init config set into the directory, ignoring overflow means that Upstart will miss some files.

Upstart will probably want to respond to queue overflow by walking the directory manually to see what it missed.

review: Needs Fixing

[Frank Ch. Eigler (fche)]

> NAK.
> This code is still not "handling" the problem, it's just ignoring it.
> [...]

It is still an improvement over crashing.

Unmerged revisions

1050. By James Hunt on 2011-06-21

Special case code to handle inotify event queue overflow (LP:#777093).

* nih/watch.c: nih_watch_reader(): Handle situation where kernel
  is generating events faster than can be consumed by simply
  ignoring them. Failure to do so results in an assertion failure in
  nih_watch_handle_by_wd() since the inotify_event object will
  contain an invalid watch descriptor (-1).
* nih/tests/test_watch.c:
  - Added PROC_MAX_INOTIFY define.
  - test_reader(): Added test to check for inotify event queue overflow
    handling.

=== modified file 'ChangeLog'
--- ChangeLog	2011-06-20 18:47:02 +0000
+++ ChangeLog	2011-06-21 16:26:00 +0000
@@ -1,3 +1,17 @@
[phone]  James Hunt  <[e-mail]>
+
+	Special case code to handle inotify event queue overflow (LP:#777093).
+	
+	* nih/watch.c: nih_watch_reader(): Handle situation where kernel
+	  is generating events faster than can be consumed by simply
+	  ignoring them. Failure to do so results in an assertion failure in
+	  nih_watch_handle_by_wd() since the inotify_event object will
+	  contain an invalid watch descriptor (-1).
+	* nih/tests/test_watch.c:
+	  - Added PROC_MAX_INOTIFY define.
+	  - test_reader(): Added test to check for inotify event queue overflow
+	    handling.
+
 2011-06-20  James Hunt  <[e-mail]>
 
 	* nih/watch.c (nih_watch_handle): Handle non-directory watches;

=== modified file 'nih/tests/test_watch.c'
--- nih/tests/test_watch.c	2011-06-20 16:09:08 +0000
+++ nih/tests/test_watch.c	2011-06-21 16:26:00 +0000
@@ -39,6 +39,7 @@
 #include <nih/error.h>
 #include <nih/logging.h>
 
+#define PROC_MAX_INOTIFY "/proc/sys/fs/inotify/max_queued_events"
 
 static int
 my_filter (void       *data,
@@ -1199,6 +1200,74 @@
 
 	nih_free (last_path);
 
+	/* only run this test if /proc mounted and inotify available */
+	if (access (PROC_MAX_INOTIFY, F_OK) == 0)
+	{
+		size_t max_queued_events = 0, i;
+
+		TEST_FEATURE ("with inotify event queue overflow");
+
+		modify_called = 0;
+		last_watch = NULL;
+		last_path = NULL;
+		last_data = NULL;
+
+		/* establish per process inotify event queue size */
+	       	fd = fopen (PROC_MAX_INOTIFY, "r");
+		TEST_NE_P (fd, NULL);
+		fscanf (fd, "%lu", (long unsigned int *)&max_queued_events);
+		fclose (fd);
+
+		fd = fopen (filename, "w");
+		fprintf (fd, "foo\n");
+		fclose (fd);
+
+		nfds = 0;
+		FD_ZERO (&readfds);
+		FD_ZERO (&writefds);
+		FD_ZERO (&exceptfds);
+
+		nih_io_select_fds (&nfds, &readfds, &writefds, &exceptfds);
+		select (nfds, &readfds, &writefds, &exceptfds, NULL);
+		nih_io_handle_fds (&readfds, &writefds, &exceptfds);
+
+		TEST_EQ (modify_called, 1);
+
+		modify_called = 0;
+
+		/* Force an inotify event queue overflow.
+		 *
+		 * Note that we cannot force max_queued_events events and
+		 * _then_ try to generate a few more later since this
+		 * test is timing critical: we want to guarantee that
+		 * userspace is flooded with inotify events such that it
+		 * cannot handle them fast enough.
+		 * */
+
+		for (i=0; i < max_queued_events + 100; ++i) {
+			fd = fopen (filename, "a+");
+			TEST_NE_P (fd, NULL);
+			fputc ('\0', fd);
+			fclose (fd);
+
+			nfds = 0;
+			FD_ZERO (&readfds);
+			FD_ZERO (&writefds);
+			FD_ZERO (&exceptfds);
+
+			nih_io_select_fds (&nfds, &readfds, &writefds, &exceptfds);
+			select (nfds, &readfds, &writefds, &exceptfds, NULL);
+			nih_io_handle_fds (&readfds, &writefds, &exceptfds);
+		}
+
+		/* If we got here, the test has passed since the original
+		 * problem caused an assertion failure in
+		 * nih_watch_handle_by_wd().
+		 */
+		TEST_GT (modify_called, 0);
+
+		nih_free (last_path);
+	}
 
 	/* Check that we can rename the file, we should get the delete
 	 * handler called followed by the create handler.

=== modified file 'nih/watch.c'
--- nih/watch.c	2011-06-20 16:09:08 +0000
+++ nih/watch.c	2011-06-21 16:26:00 +0000
@@ -424,6 +424,13 @@
 		if (len < sz)
 			goto finish;
 
+		/* Handle situation where kernel is generating events faster
+		 * than can be consumed. Inotify will (somewhat ironically)
+		 * generate an event to signify this, so ignore it.
+		 */
+		if (event->mask & IN_Q_OVERFLOW)
+			goto consume;
+
 		/* Find the handle for this watch */
 		handle = nih_watch_handle_by_wd (watch, event->wd);
 		if (handle)
@@ -437,6 +444,7 @@
 		if (caught_free)
 			return;
 
+consume:
 		/* Remove the event from the front of the buffer, and
 		 * decrease our own length counter.
 		 */

=== modified file 'po/libnih.pot'
--- po/libnih.pot	2010-12-23 22:04:08 +0000
+++ po/libnih.pot	2011-06-21 16:26:00 +0000
@@ -6,9 +6,9 @@
 #, fuzzy
 msgid ""
 msgstr ""
-"Project-Id-Version: libnih 1.0.3\n"
+"Project-Id-Version: libnih 1.0.4\n"
 "Report-Msgid-Bugs-To: [e-mail]\n"
-"POT-Creation-Date: 2010-12-23 21:53+0000\n"
+"POT-Creation-Date: 2011-06-21 16:53+0100\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"
@@ -158,7 +158,7 @@
 msgid "Options:\n"
 msgstr ""
 
-#: nih/watch.c:550
+#: nih/watch.c:567
 msgid "Unable to watch directory"
 msgstr ""