bug#21337: 25.0.50; inotify error message

[Robert Pluim]

Eli Zaretskii <eliz@gnu.org> writes:

>> is that the read will succeed, however to_read is very often 0, so
>> it's not surprising the read fails.
>
> That's strange, isn't it?  If to_read is zero, how come pselect
> indicated that FD has stuff to be read?

pselect didn't, but we fooled ourselves into thinking it did.

I suspected this was down to Gnus tickling some file descriptor
handling bug, and since the only code I could see in
wait_reading_process_output() that looked like it might be messing
things up was under a HAVE_GNUTLS define, I rebuilt without GnuTLS,
and the problem went away. After debugging some more, here's my
current theory (look around line 4904 of process.c:

1. the file descriptor for inotify events is checked by xg_select
   (since HAVE_GLIB == 1) using the Available fd_set
2. there is nothing to be read, so xg_select returns nfds =
   0. However, the inotify FD is still set in Available, which is
   unexpected but not wrong.
3. We then run:

                  for (channel = 0; channel < FD_SETSIZE; ++channel)
                    if (! NILP (chan_process[channel]))
                      {
                        struct Lisp_Process *p =
                          XPROCESS (chan_process[channel]);
                        if (p && p->gnutls_p && p->gnutls_state
                            && ((emacs_gnutls_record_check_pending
                                 (p->gnutls_state))
                                > 0))
                          {
                            nfds++;
                            eassert (p->infd == channel);
                            FD_SET (p->infd, &Available);
                          }
                      }

   which checks if any of the TLS file descriptors have waiting data
   that hasn't been indicated by xg_select, and sets the corresponding
   bit in the Available fd_set. In my case one of them does, so we
   increment nfds
   
4. We then reach

      if (no_avail || nfds == 0)
        continue;

      for (channel = 0; channel <= max_input_desc; ++channel)
        {
          struct fd_callback_data *d = &fd_callback_info[channel];
          if (d->func
              && ((d->condition & FOR_READ
                   && FD_ISSET (channel, &Available))
                  || (d->condition & FOR_WRITE
                      && FD_ISSET (channel, &write_mask))))
            d->func (channel, d->data);
        }

   around line 5085 and loop over all the FDs we know about,
   checking to see if any of them are set in the Available fd_set

5. the inotify FD is still set in Available, so we call its callback
   function, which tries to read data when it shouldn't

Step 5 should never happen, but because step 2 did not clear the
Available fd_set, and the HAVE_GNUTLS code incremented nfds, we think
there's data to be read.

The following patch, which zeros out Available when the GNUTLS code
does its thing has fixed things for me. I've convinced myself it has
no bad side-effects, but would appreciate a second opinion, especially
since the option also exists to just zero out Available
unconditionally when nfds==0.

diff --git a/src/process.c b/src/process.c
index 9d8fa22..dcc004e 100644
--- a/src/process.c
+++ b/src/process.c
@@ -4913,6 +4913,10 @@ wait_reading_process_output (intmax_t time_limit, int 
nsecs, int read_kbd,
              data is available in the buffers manually.  */
           if (nfds == 0)
            {
+             fd_set tls_available;
+             int set = 0;
+
+             FD_ZERO (&tls_available);
              if (! wait_proc)
                {
                  /* We're not waiting on a specific process, so loop
@@ -4933,7 +4937,8 @@ wait_reading_process_output (intmax_t time_limit, int 
nsecs, int read_kbd,
                          {
                            nfds++;
                            eassert (p->infd == channel);
-                           FD_SET (p->infd, &Available);
+                           FD_SET (p->infd, &tls_available);
+                           set++;
                          }
                      }
                }
@@ -4950,9 +4955,12 @@ wait_reading_process_output (intmax_t time_limit, int 
nsecs, int read_kbd,
                      nfds = 1;
                      eassert (0 <= wait_proc->infd);
                      /* Set to Available.  */
-                     FD_SET (wait_proc->infd, &Available);
+                     FD_SET (wait_proc->infd, &tls_available);
+                     set++;
                    }
                }
+             if (set)
+               Available = tls_available;
            }
 #endif
        }