INN 2.5.4 strange crash

[Petr Novopashenniy]

On Wed, 21 Jan 2015, Julien ?LIE wrote:

J?? Hi Petr,
J?? 
J?? >> Are you still regularly seeing segfaults?
J?? > 
J?? > Yes, frequently:
J?? > 
J?? > ftp://ftp.neva.ru/tmp/innd_check.log
J?? 
J?? Still when news.daily runs (as you already told us).

Yes.

J?? 
J?? 
J?? >> No line like
J?? >>   "%s number of descriptors (%d) exceeding or equaling FD_SETSIZE (%d)"
J?? >>   "free but was in WMASK"
J?? >>   "free but was in RMASK"
J?? >> in your news logs?
J?? > 
J?? > Hardly ever - "free:-1 20 free but was in SMASK"
J?? 
J?? Which indicates there is somewhere a bug in channel handling.
J?? For a reason I still do not know, a sleeping channel is closed without updating
J?? its state in the global variable that tracks the currently opened sleeping channels.
J?? 
J?? 
J?? >> Well, though cp->Waker should not be NULL at that line, could you please try
J?? >> the following change and tell us whether the issue is still present?
J?? > 
J?? > Thanks, Julien, I apply this patch and wait now...
J?? 
J?? I hope it will solve the segfaults.

No crashes after 2 day run.

J?? 
J?? Incidentally, I have a better patch that will log "%s %d sleeping without Waker"
J?? when the issue occurs.  It will normally not segfault.
J?? I also fixed a bug in the count of sleeping channels; it was not always updated,
J?? and the newly highest file descriptor related to a sleeping channel wasn't also
J?? always updated.
J?? 
J?? If you could use that patch and report us occurrences of "%s %d sleeping without Waker",
J?? it would be great.  It will maybe give a hint about what is happening.
J?? 
J?? 
J?? --- chan.c	(r?vision 9776)
J?? +++ chan.c	(copie de travail)
J?? @@ -571,6 +571,21 @@
J??  
J??  
J??  /*
J?? +**  When removing a channel from the sleep mask, we want to lower the last
J?? +**  file descriptor if we removed the highest one.  Called from SCHANremove.
J?? +*/
J?? +static void
J?? +CHANresetlastsleeping(int fd)
J?? +{
J?? +    if (fd == channels.max_sleep_fd) {
J?? +        while (   !FD_ISSET(channels.max_sleep_fd, &channels.sleep_set)
J?? +               && channels.max_sleep_fd > 1)
J?? +            channels.max_sleep_fd--;
J?? +    }
J?? +}
J?? +
J?? +
J?? +/*
J??  **  Mark a channel as an active reader.
J??  */
J??  void
J?? @@ -629,8 +644,6 @@
J??  void
J??  SCHANremove(CHANNEL *cp)
J??  {
J?? -    int fd;
J?? -
J??      if (!CHANsleeping(cp))
J??          return;
J??      FD_CLR(cp->fd, &channels.sleep_set);
J?? @@ -638,12 +651,7 @@
J??      cp->Waketime = 0;
J??  
J??      /* If this was the highest descriptor, get a new highest. */
J?? -    if (cp->fd == channels.max_sleep_fd) {
J?? -        fd = channels.max_sleep_fd;
J?? -        while (!FD_ISSET(fd, &channels.sleep_set) && fd > 1)
J?? -            fd--;
J?? -        channels.max_sleep_fd = fd;
J?? -    }
J?? +    CHANresetlastsleeping(cp->fd);
J??  }
J??  
J??  
J?? @@ -1276,12 +1284,18 @@
J??                  if (cp->Type == CTfree) {
J??                      warn("%s %d free but was in SMASK", CHANname(cp), fd);
J??                      FD_CLR(fd, &channels.sleep_set);
J?? +                    channels.sleep_count--;
J?? +                    CHANresetlastsleeping(fd);
J??                      close(fd);
J??                      cp->fd = -1;
J??                  } else {
J??                      cp->LastActive = Now.tv_sec;
J??                      SCHANremove(cp);
J?? -                    (*cp->Waker)(cp);
J?? +                    if (cp->Waker != NULL) {
J?? +                        (*cp->Waker)(cp);
J?? +                    } else {
J?? +                        warn("%s %d sleeping without Waker", CHANname(cp), fd);
J?? +                    }
J??                  }
J??              }
J??  

I apply your patch (from your private mail, for INN 2.5.4, I see two new 
lines channels.sleep_count--; and CHANresetlastsleeping(fd); ), 
and waiting results.

Thanks, Julien!

--pety