bug#10044: "warning: reader_thread.SetEvent failed with 6 for fd -1" and accessing fd_info[-1]

bug#10044: "warning: reader_thread.SetEvent failed with 6 for fd -1" and accessing fd_info[-1]

[Juanma Barranquero]

Package: emacs,w32
Severity: minor

  (gdb) run -Q --eval "(shell-command \"dir\")"
  Starting program: c:\emacs\debug\bin\emacs.exe -Q --eval
"(shell-command \"dir\")"
  [New Thread 5948.0x17a4]
  [New Thread 5948.0x9c8]
  [New Thread 5948.0x4b4]
  [New Thread 5948.0x16ac]
  warning: reader_thread.SetEvent failed with 6 for fd -1

which can not be right, because it happens in this bit of code of
w32proc.c:reader_thread():

  /* Our identity */
  cp = (child_process *)arg;

  /* We have to wait for the go-ahead before we can start */
  if (cp == NULL
      || WaitForSingleObject (cp->char_consumed, INFINITE) != WAIT_OBJECT_0)
    return 1;

  for (;;)
    {
      int rc;

      if (fd_info[cp->fd].flags & FILE_LISTEN)
	rc = _sys_wait_accept (cp->fd);
      else
	rc = _sys_read_ahead (cp->fd);

      /* The name char_avail is a misnomer - it really just means the
	 read-ahead has completed, whether successfully or not. */
      if (!SetEvent (cp->char_avail))
        {
	  DebPrint (("reader_thread.SetEvent failed with %lu for fd %ld\n",
		     GetLastError (), cp->fd));
	  return 1;
	}

   [...]

which means that cp->fd is being used as an index into fd_info[], and
the choice between _sys_wait_accept and _sys_read_ahead is bogus.

IIUC, cp->fd == -1 means that the wait was intended, but no input is
expected, so I think the following patch is enough:


=== modified file 'src/w32proc.c'
--- src/w32proc.c       2011-06-24 21:25:22 +0000
+++ src/w32proc.c       2011-11-14 15:19:09 +0000
@@ -241,7 +241,8 @@

   /* We have to wait for the go-ahead before we can start */
   if (cp == NULL
-      || WaitForSingleObject (cp->char_consumed, INFINITE) != WAIT_OBJECT_0)
+      || WaitForSingleObject (cp->char_consumed, INFINITE) != WAIT_OBJECT_0
+      || cp->fd < 0)
     return 1;

   for (;;)

bug#10044: "warning: reader_thread.SetEvent failed with 6 for fd -1" and accessing fd_info[-1]

[Eli Zaretskii]

> From: Juanma Barranquero <lekktu@gmail.com>
> Date: Mon, 14 Nov 2011 16:33:42 +0100
> 
> which means that cp->fd is being used as an index into fd_info[], and
> the choice between _sys_wait_accept and _sys_read_ahead is bogus.
> 
> IIUC, cp->fd == -1 means that the wait was intended, but no input is
> expected, so I think the following patch is enough:
> 
> 
> === modified file 'src/w32proc.c'
> --- src/w32proc.c       2011-06-24 21:25:22 +0000
> +++ src/w32proc.c       2011-11-14 15:19:09 +0000
> @@ -241,7 +241,8 @@
> 
>    /* We have to wait for the go-ahead before we can start */
>    if (cp == NULL
> -      || WaitForSingleObject (cp->char_consumed, INFINITE) != WAIT_OBJECT_0)
> +      || WaitForSingleObject (cp->char_consumed, INFINITE) != WAIT_OBJECT_0
> +      || cp->fd < 0)
>      return 1;
> 
>    for (;;)

Let's go for it, thanks.

Does this patch by chance solve a similar bogus DebPrint message each
time a versioned file under bzr control is visited?

bug#10044: "warning: reader_thread.SetEvent failed with 6 for fd -1" and accessing fd_info[-1]

[Juanma Barranquero]

On Mon, Nov 14, 2011 at 18:14, Eli Zaretskii <eliz@gnu.org> wrote:

> Let's go for it, thanks.

OK.

> Does this patch by chance solve a similar bogus DebPrint message each
> time a versioned file under bzr control is visited?

From a brief test, I'd say yes.

    Juanma

bug#10044: "warning: reader_thread.SetEvent failed with 6 for fd -1" and accessing fd_info[-1]

[Juanma Barranquero]

On Mon, Nov 14, 2011 at 18:30, Juanma Barranquero <lekktu@gmail.com> wrote:

>> Let's go for it, thanks.

Done, closing.