directory change notification patch

[Derrell.Lipman at UnwiredUniverse.com]

Jeremy Allison <jra at samba.org> writes:

> Whenever a kernel change notify signal has been sent, a byte is written
> down the select internal pipe. Thus, the next call to sys_select() will
> *always* return -1 and EINTR for every queued real-time signal that was
> processed by the change notify signal handler. The signals are essentiall
> being queued in the pipe buffer (usually at least 256 bytes, so 256 signals
> which should be more than enough for Samba).
>
> The change notify signals are POSIX real-time, they are queued and
> not lost. Can you explain to me how you think a change notify signal
> can ever get lost with our current code ?
>
> Please let me know why this logic is in error. I'll need to see
> an explicit code-path/timing walkthrough to be convinced.

Here's what I saw when I read Mark's explanation:

>From sys_select():

	maxfd = MAX(select_pipe[0]+1, maxfd);

	/* If readfds is NULL we need to provide our own set. */
	if (readfds) {
		readfds2 = readfds;
	} else {
		readfds2 = &readfds_buf;
		FD_ZERO(readfds2);
	}
	FD_SET(select_pipe[0], readfds2);

	errno = 0;
	ret = select(maxfd,readfds2,writefds,errorfds,tval);

Let's say we're passed readfds with 1 file descriptor to read from, i.e. one
bit set.  readfds2 is assigned readfds, and then the pipe file descriptor is
added to readfds2.  There are now two file descriptors specified in readfds2.
assume for the moment that nothing is specified in writefds nor errorfds.

select() is called when data is available on the data file descriptor provided
by the user AND when there has been a signal so the pipe fd has data
available.  select() will return 2 because there is data available on two file
descriptors: the user fd and the pipe fd.

	if (ret <= 0) {
		FD_ZERO(readfds2);
		if (writefds)
			FD_ZERO(writefds);
		if (errorfds)
			FD_ZERO(errorfds);
	}

ret is 2, so that code is irrelevant.

	if (FD_ISSET(select_pipe[0], readfds2)) {
		char c;
		saved_errno = errno;
		if (read(select_pipe[0], &c, 1) == 1) {
			pipe_read++;
		}

Ok now we've retrieved the indicator for the signal, but...

		errno = saved_errno;
		FD_CLR(select_pipe[0], readfds2);
		ret--;

ret is now 1.

		if (ret == 0) {

this code is NOT entered, so ret is not set to -1, and errno is not set to
EINTR.  Only in the case where no data was available on the user-specified fd
would we enter this block.

			ret = -1;
			errno = EINTR;
		}
	}

        return ret;

we just returned 1, not -1, and since we read from select_pipe[0], the
indication of that signal has been consumed and lost forever.

Am I missing something here?

Derrell