[Click] [PATCH] do not access deleted timers

[Nadi Sarrar]

Hi Eddie,

> [..] Please try
> 
> http://www.read.cs.ucla.edu/gitweb?p=click;a=commit;h=91470b0740c7a019680b9362f15cdab96f7b08c0
> 
> and let me know if you still have problems.

Actually, yes, the attached patch fixes it. You forgot to set _schedpos1 to zero
when running timers from the runchunk list, which makes Click crash because in
Timer::unschedule(), an invalid access to the runchunk list may happen.

Thanks for fixing this in Click so fast!

Nadi


On Sun, Apr 12, 2009 at 06:18:06PM -0700, Eddie Kohler wrote:
> Hi Nadi,
> 
> Great find, and thanks very much for the fix!  We don't tend to use Click 
> with highly dynamic timer sets, so bugs are still around.  Your code looks 
> good, but unhappy with the extra char allocations, I tried to fix it in a 
> different way.  Please try
> 
> http://www.read.cs.ucla.edu/gitweb?p=click;a=commit;h=91470b0740c7a019680b9362f15cdab96f7b08c0
> 
> and let me know if you still have problems.  Again, thanks for the 
> debugging and patch.
> 
> Eddie
> 
> 
> Nadi Sarrar wrote:
> >Hi,
> >
> >a problem with the current timer implementation in Click leads to 
> >execution of
> >unscheduled timers or access to already deleted timers, which can result 
> >in any
> >kind of unexpected behavior including segmentation faults at unrelated code
> >sections, which made it fun to debug :) Possible patch attached.
> >
> >The problem occurs only under certain conditions:
> >
> >  1) A lot of timers (more than max_timers = 64) must be used that are
> >     scheduled with close expiration times.
> >  2) Timer handler functions delete (or unschedule) other timers.
> >  3) High load helps to trigger this bug fast.
> >
> >The problematic code section is in Master::run_timers(), right after the 
> >usual
> >handling of scheduled timers. There, all scheduled and expired timers are 
> >taken
> >out of the heap, stored in a vector and then run sequentially, while 
> >missing a
> >check whether the current timer is still alive (could be 
> >deleted/unscheduled by
> >one of the previously run timers).
> >
> >Nadi
> >
> >
> >
> >------------------------------------------------------------------------
> >
> >_______________________________________________
> >click mailing list
> >click at amsterdam.lcs.mit.edu
> >https://amsterdam.lcs.mit.edu/mailman/listinfo/click
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: click-timer-runchunk.patch
Type: text/x-diff
Size: 598 bytes
Desc: not available
Url : http://amsterdam.lcs.mit.edu/pipermail/click/attachments/20090413/81075c83/attachment.patch