buglwIP - A Lightweight TCP/IP stack - Bugs: bug #48568, timeouts does not support late...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #48568: timeouts does not support late firing properly

Submitted by:  Sylvain Rochet <gradator>
Submitted on:  Wed 20 Jul 2016 10:07:28 AM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: 2.0.0 Beta2
lwIP version: git head

Wed 27 Jul 2016 07:28:23 PM UTC, comment #4:

I've fixed this bug, can we continue the diff-time vs. absolute-time in a seperate task, or reuse patch #7855?

Hey, I've tried lwIP on an ATmega just ~2 years ago, andi t worked! (OK, I only had a slipif and needed a slipif-to-ethernet router on my PC...) But that was no a real-world thing, of course... ;-)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 27 Jul 2016 07:20:29 PM UTC, comment #3:

Absolute timers are great, and faster, because the comparison is obviously the most simple comparison that could be made, well, at least, on 32-bit targets (are there still users of lwIP on 8-bit platforms ?, that doesn't sound like a good design choice nowadays ;-), I'm pretty none of us tried that for a long time).

But, there is always a but, it means we need a new u32_t field in the timeout struct to store the target time, so we can keep the timeout period, we need that in order to be able to restart all timers on wakeup event for devices that sleep, this is what the timeouts_last_time is currently used to. With delta-diff timers we only need one variable, with absolute timers, we need one "timeouts_last_time"-equivalent per timer.

Sylvain Rochet <gradator>
Project Member
Wed 27 Jul 2016 07:06:17 PM UTC, comment #2:

> This could be fixed by moving to absolute target timers


Given the fact that less-than-32 bit platforms get more uncommon, the arguments for delta-diff timers get less pressing, indeed ;-)

I'd be curious to see a diff to support absolute times. With correct refactoring, it shouldn't be too messy?

We do have patch #7855, but this is far from being a small diff...

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 20 Jul 2016 10:09:19 AM UTC, comment #1:

This could be fixed by moving to absolute target timers instead of the currently used delta-diff timers (which I dislike, due to the problem mentioned below, at least). The main problem is our current support of timers restart on wake up event for devices that sleep to prevent firing all timers at once, that's hard to achieve with absolute target timers.

Or maybe we can find a hack about the timeouts_last_time to take into account the extra period if we are able to detect it.

Sylvain Rochet <gradator>
Project Member
Wed 20 Jul 2016 10:07:28 AM UTC, original submission:

With the current code, timeouts get delayed when we are in a debugger breakpoint and sys_now() might continues running while we are stopped (e.g. when it uses a clock source from an OS which does not stop).

sys_check_timeouts() increases timeouts_last_time only by the expired timeout's time. If that handler restarts a timeout (as is the case for all lwIP-internal timers), it gets an addditional delay of the elapsed time in the debugger stop period, meaning if we stopped for 2 minutes, the timer will get an extra 2 minutes delay.

Sylvain Rochet <gradator>
Project Member

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by gradator (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 27 Jul 2016 07:28:23 PM UTCgoldsimonStatusNone=>Fixed
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed

    Back to the top


    Powered by Savane 3.1-cleanup1