all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Robert Pluim <rpluim@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 21337@debbugs.gnu.org
Subject: bug#21337: 25.0.50; inotify error message
Date: Wed, 26 Aug 2015 19:02:23 +0200	[thread overview]
Message-ID: <87twrm2db4.fsf@gmail.com> (raw)
In-Reply-To: <83si78fpi5.fsf@gnu.org> (Eli Zaretskii's message of "Mon, 24 Aug 2015 22:36:02 +0300")

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
 	}





  reply	other threads:[~2015-08-26 17:02 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-24 12:12 bug#21337: 25.0.50; inotify error message Robert Pluim
2015-08-24 15:52 ` Eli Zaretskii
2015-08-24 16:22   ` Robert Pluim
2015-08-24 16:34     ` Eli Zaretskii
2015-08-24 16:51       ` Robert Pluim
2015-08-24 17:18         ` Eli Zaretskii
2015-08-24 18:32           ` Robert Pluim
2015-08-24 19:36             ` Eli Zaretskii
2015-08-26 17:02               ` Robert Pluim [this message]
2015-08-28 13:30                 ` Eli Zaretskii
2015-09-05  8:38                   ` Eli Zaretskii
2015-09-07  7:21                 ` Tassilo Horn
2015-09-07  7:43                   ` Robert Pluim
2015-09-07 15:01                     ` Eli Zaretskii
2015-09-07 15:15                       ` Robert Pluim
2015-09-09 16:49                         ` Tassilo Horn
2015-09-09 17:57                           ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87twrm2db4.fsf@gmail.com \
    --to=rpluim@gmail.com \
    --cc=21337@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.