INN 2.5.4 strange crash

[Julien ÉLIE]

Hi Petr,

>> Are you still regularly seeing segfaults?
> 
> Yes, frequently:
> 
> ftp://ftp.neva.ru/tmp/innd_check.log

Still when news.daily runs (as you already told us).


>> No line like
>>   "%s number of descriptors (%d) exceeding or equaling FD_SETSIZE (%d)"
>>   "free but was in WMASK"
>>   "free but was in RMASK"
>> in your news logs?
> 
> Hardly ever - "free:-1 20 free but was in SMASK"

Which indicates there is somewhere a bug in channel handling.
For a reason I still do not know, a sleeping channel is closed without updating
its state in the global variable that tracks the currently opened sleeping channels.


>> Well, though cp->Waker should not be NULL at that line, could you please try
>> the following change and tell us whether the issue is still present?
> 
> Thanks, Julien, I apply this patch and wait now...

I hope it will solve the segfaults.

Incidentally, I have a better patch that will log "%s %d sleeping without Waker"
when the issue occurs.  It will normally not segfault.
I also fixed a bug in the count of sleeping channels; it was not always updated,
and the newly highest file descriptor related to a sleeping channel wasn't also
always updated.

If you could use that patch and report us occurrences of "%s %d sleeping without Waker",
it would be great.  It will maybe give a hint about what is happening.


--- chan.c	(révision 9776)
+++ chan.c	(copie de travail)
@@ -571,6 +571,21 @@
 
 
 /*
+**  When removing a channel from the sleep mask, we want to lower the last
+**  file descriptor if we removed the highest one.  Called from SCHANremove.
+*/
+static void
+CHANresetlastsleeping(int fd)
+{
+    if (fd == channels.max_sleep_fd) {
+        while (   !FD_ISSET(channels.max_sleep_fd, &channels.sleep_set)
+               && channels.max_sleep_fd > 1)
+            channels.max_sleep_fd--;
+    }
+}
+
+
+/*
 **  Mark a channel as an active reader.
 */
 void
@@ -629,8 +644,6 @@
 void
 SCHANremove(CHANNEL *cp)
 {
-    int fd;
-
     if (!CHANsleeping(cp))
         return;
     FD_CLR(cp->fd, &channels.sleep_set);
@@ -638,12 +651,7 @@
     cp->Waketime = 0;
 
     /* If this was the highest descriptor, get a new highest. */
-    if (cp->fd == channels.max_sleep_fd) {
-        fd = channels.max_sleep_fd;
-        while (!FD_ISSET(fd, &channels.sleep_set) && fd > 1)
-            fd--;
-        channels.max_sleep_fd = fd;
-    }
+    CHANresetlastsleeping(cp->fd);
 }
 
 
@@ -1276,12 +1284,18 @@
                 if (cp->Type == CTfree) {
                     warn("%s %d free but was in SMASK", CHANname(cp), fd);
                     FD_CLR(fd, &channels.sleep_set);
+                    channels.sleep_count--;
+                    CHANresetlastsleeping(fd);
                     close(fd);
                     cp->fd = -1;
                 } else {
                     cp->LastActive = Now.tv_sec;
                     SCHANremove(cp);
-                    (*cp->Waker)(cp);
+                    if (cp->Waker != NULL) {
+                        (*cp->Waker)(cp);
+                    } else {
+                        warn("%s %d sleeping without Waker", CHANname(cp), fd);
+                    }
                 }
             }
 



-- 
Julien ÉLIE

« Même avec Dieu, il ne faut pas tenter le Diable. » (Raymond
  Devos)