irpd core dumps

[Paul A Vixie]

> After further investigation I found this to be a problem with the way idle
> timers were cleaned up.  In the case of irpd, the timed out idle_timer was
> freed twice which cause the dump I reported in the previouse message.

i see the problem you're describing.

> I propose the following change:
> 
> *** ev_timers.c Wed Sep 29 14:31:11 1999
> --- /stuff/archives/tmp/src/lib/isc/ev_timers.c Fri Jan  8 14:25:29 1999
> ***************
> *** 393,399 ****
>         if (evCmpTime(idle, this->max_idle) >= 0) {
>                 (this->func)(opaqueCtx, this->uap, this->timer->due,
>                              this->max_idle);
> !               /* at this point idle_timer is removed and the interval is 0 */
>         } else {
>                 /* evDrop() will reschedule the timer. */
>                 this->timer->inter = evSubTime(this->max_idle, idle);
> --- 393,404 ----
>         if (evCmpTime(idle, this->max_idle) >= 0) {
>                 (this->func)(opaqueCtx, this->uap, this->timer->due,
>                              this->max_idle);
> !               /*
> !                * Setting the interval to zero will cause the timer to
> !                * be cleaned up in evDrop().
> !                */
> !               this->timer->inter = evConsTime(0, 0);
> !               FREE(this);
>         } else {
>                 /* evDrop() will reschedule the timer. */
>                 this->timer->inter = evSubTime(this->max_idle, idle);

i agree that calling FREE() here is a mistake, but since evDrop() looks
for the x==0 case rather than the x>y case, we still have to zero it out.

therefore i propose:

Index: CHANGES
===================================================================
RCS file: /proj/cvs/isc/bind/src/CHANGES,v
retrieving revision 8.437
diff -u -r8.437 CHANGES
--- CHANGES     1999/10/06 22:42:40     8.437
+++ CHANGES     1999/10/06 22:59:54
@@ -1,3 +1,5 @@
+ 827.  [bug]           idle timers could be freed twice (seen in irpd.)
+
  826.  [contrib]       various improvements to contrib/dns_signer (from TIS).
 
  825.  [support]       change __NAMESER and __RES to 19991006.
Index: lib/isc/ev_timers.c
===================================================================
RCS file: /proj/cvs/isc/bind/src/lib/isc/ev_timers.c,v
retrieving revision 1.24
diff -u -r1.24 ev_timers.c
--- lib/isc/ev_timers.c 1999/01/08 19:25:29     1.24
+++ lib/isc/ev_timers.c 1999/10/06 22:59:24
@@ -383,8 +383,8 @@
 idle_timeout(evContext opaqueCtx,
             void *uap,
             struct timespec due,
-            struct timespec inter
-) {
+            struct timespec inter)
+{
        evContext_p *ctx = opaqueCtx.opaque;
        idle_timer *this = uap;
        struct timespec idle;
@@ -393,14 +393,10 @@
        if (evCmpTime(idle, this->max_idle) >= 0) {
                (this->func)(opaqueCtx, this->uap, this->timer->due,
                             this->max_idle);
-               /*
-                * Setting the interval to zero will cause the timer to
-                * be cleaned up in evDrop().
-                */
+               /* This tells evDrop() to clear the timer. */
                this->timer->inter = evConsTime(0, 0);
-               FREE(this);
        } else {
-               /* evDrop() will reschedule the timer. */
+               /* This tells evDrop() to reset the timer. */
                this->timer->inter = evSubTime(this->max_idle, idle);
        }
 }

comments?