lwIP - A Lightweight TCP/IP stack - Bugs: bug #19167, tcp timeout handler can cause...
You are not allowed to post comments on this tracker with your current authentication level.
bug #19167: tcp timeout handler can cause crash being invoked by sys_sem_wait()
Submitter: | Dmitry Potapov <dpotapov> | ||
Submitted: | Wed 28 Feb 2007 12:20:46 AM UTC | ||
Category: | TCP | Severity: | 3 - Normal |
Item Group: | Crash Error | Status: | Fixed |
Privacy: | Public | Assigned to: | goldsimon |
Open/Closed: | Closed | Planned Release: | None |
lwIP version: | None |
Jump to the original submission
Wed 16 May 2007 11:02:52 AM UTC, comment #33: |
Simon Goldschmidt <goldsimon> |
Wed 16 May 2007 09:29:19 AM UTC, comment #32: Ok, seems good for me...
|
Frédéric Bernon <fbernon> |
Wed 16 May 2007 09:27:15 AM UTC, comment #31:
|
Simon Goldschmidt <goldsimon> |
Wed 16 May 2007 09:16:01 AM UTC, comment #30: So, to be sure, you use your patch file #12142 ? You just change memp.c ?
|
Frédéric Bernon <fbernon> |
Wed 16 May 2007 09:10:27 AM UTC, comment #29: From your description, that seems like a good idea, but I haven't looked into it in any more detail than that! |
Kieran Mansley <kieranm> |
Wed 16 May 2007 07:43:11 AM UTC, comment #28: I already proposed this in another thread, but it belongs here:
|
Simon Goldschmidt <goldsimon> |
Thu 12 Apr 2007 09:03:50 AM UTC, comment #27: Regarding the timestamps: I just realized there is a function sys_jiffies() (defined in sys.h, should be included in sys_arch.c), which is extgensively used in PPP.
|
Simon Goldschmidt <goldsimon> |
Fri 09 Mar 2007 08:15:39 AM UTC, comment #26: OK, I'll work on this offline, or should I try to create a new CVS branch? Never worked with that, though. I only know SVN...
|
Simon Goldschmidt <goldsimon> |
Wed 07 Mar 2007 01:10:35 PM UTC, comment #25: Thanks for working on this, and good to see patches coming forward.
|
Kieran Mansley <kieranm> |
Wed 07 Mar 2007 12:45:35 PM UTC, comment #24: I think it is better to define them as macros
|
Dmitry Potapov <dpotapov> |
Wed 07 Mar 2007 12:12:26 PM UTC, comment #23: Sorry for the mess: I simply forgot the return value in sys_sem_wait_timeout().
|
Simon Goldschmidt <goldsimon> |
Wed 07 Mar 2007 11:45:31 AM UTC, comment #22: Here comes the patch. It completely removes timeout timer handling from semaphores. As a result of this, sys_sem_wait() and sys_sem_wait_timeout() basicly become direct calls to sys_arch_sem_wait().
|
Simon Goldschmidt <goldsimon> |
Mon 05 Mar 2007 12:59:38 PM UTC, comment #21: Send me the patch and I'll do some black box type testing on it. |
Thomas Taranowski <taranowski> |
Mon 05 Mar 2007 12:05:14 PM UTC, comment #20: Sorry - didn't mean to stop you progressing on this, just wanted to let you know where I stood, and to ask that you take particular care on this one! |
Kieran Mansley <kieranm> |
Mon 05 Mar 2007 11:59:52 AM UTC, comment #19: So what do we do?
|
Simon Goldschmidt <goldsimon> |
Mon 05 Mar 2007 11:52:24 AM UTC, comment #18: I haven't had time to follow this discussion yet, sorry, but a fairly fundamental change such as this will need some careful thought and testing before going in. I agree that something needs to be done, but at the moment I'm not sure what the something is. |
Kieran Mansley <kieranm> |
Mon 05 Mar 2007 11:43:35 AM UTC, comment #17: If the other developers don't mind, I'll take a look at this. I think lwip-users summarized it to taking timers (aka timeouts) off the semaphores, so that timeouts only can occur when waiting for an mbox.
|
Simon Goldschmidt <goldsimon> |
Thu 01 Mar 2007 01:34:24 PM UTC, comment #16: Yes, I assumed that timeout must be less than MAX_VALUE(timediff_t) , i.e. in our case. it is about 24 days. I am sorry that I did not state that explicitly. |
Dmitry Potapov <dpotapov> |
Thu 01 Mar 2007 01:26:01 PM UTC, comment #15: Sorry for posting again, but I just found out that it seems to work if the maximum timeout is 0x7fffffff (e.g. MAX_VALUE(timediff_t)) |
Simon Goldschmidt <goldsimon> |
Thu 01 Mar 2007 01:21:23 PM UTC, comment #14: Hi Dimitry,
|
Simon Goldschmidt <goldsimon> |
Thu 01 Mar 2007 01:11:28 PM UTC, comment #13: I have prepared a new patch that fixes both timeout handling and removes timeouts handling from sys_sem_wait.
|
Dmitry Potapov <dpotapov> |
Thu 01 Mar 2007 11:32:13 AM UTC, comment #12: Timer overflow:
|
Dmitry Potapov <dpotapov> |
Thu 01 Mar 2007 09:13:16 AM UTC, comment #11: what do you think about removing timeouts from sys_sem_wait? |
Andreas Becker <andreas_becker> |
Thu 01 Mar 2007 09:12:07 AM UTC, comment #10: Hi Dimitry,
|
Andreas Becker <andreas_becker> |
Wed 28 Feb 2007 10:39:59 PM UTC, comment #9: Another problem Andreas Becker's proposal is that timeout handler can be invoked before the actual timeout expired. Here is what happens:
|
Dmitry Potapov <dpotapov> |
Wed 28 Feb 2007 08:56:35 PM UTC, comment #8: Hi Andreas,
|
Dmitry Potapov <dpotapov> |
Wed 28 Feb 2007 08:01:14 PM UTC, comment #7: Hi,
|
Andreas Becker <andreas_becker> |
Wed 28 Feb 2007 03:32:40 PM UTC, comment #6: I have prepared a patch that replaces 'time' in timeouts with 'expires' (see attachment). It is very simple and the actual size of code has been reduced. |
Dmitry Potapov <dpotapov> |
Wed 28 Feb 2007 03:05:55 PM UTC, comment #5: Sorry for my word time-stamp. I don't want to use absolute timeouts, because it will be hard to handle if the systime is later than ->expires. Blocked for a (long) time...
|
Andreas Becker <andreas_becker> |
Wed 28 Feb 2007 11:45:19 AM UTC, comment #4: Using of timestamps instead of time-delta does NOT require 64-bit.
|
Dmitry Potapov <dpotapov> |
Wed 28 Feb 2007 10:56:50 AM UTC, comment #3: Executing timeouts only from queues might solve the problem, but it would be more clear if you could tell the code when you are in a state where you can execute timeouts.
|
Simon Goldschmidt <goldsimon> |
Wed 28 Feb 2007 10:35:13 AM UTC, comment #2: Hi all,
|
Andreas Becker <andreas_becker> |
Wed 28 Feb 2007 08:15:03 AM UTC, comment #1: Seems like SYS_LIGHTWEIGHT_PROT == 0 does some strange things not only in pbuf.c (patch #5453) ... |
Simon Goldschmidt <goldsimon> |
Wed 28 Feb 2007 12:20:46 AM UTC, original submission:
lwIP core is not re-entrable, so it is used inside one devoted thread. However, it uses memory allocation functions:
|
Dmitry Potapov <dpotapov> |
Depends on the following items: None found
Items that depend on this one: None found
There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.
Follow 13 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2007-05-16 | goldsimon | Status | Postponed | Fixed | |
Open/Closed | Open | Closed | |||
2007-03-09 | goldsimon | Status | In Progress | Postponed | |
2007-03-07 | goldsimon | Attached File | - | Added sys.c.patch, #12142 | |
2007-03-07 | goldsimon | Attached File | - | Added sys.c.patch, #12141 | |
Carbon-Copy | - | Added -email is unavailable- | |||
2007-03-05 | goldsimon | Status | None | In Progress | |
Assigned to | None | goldsimon | |||
2007-03-01 | dpotapov | Attached File | - | Added timeout-hanlder-fix.patch, #12081 | |
2007-02-28 | andreas_becker | Attached File | - | Added sys_sem_no_timeouts.sys_arch.h.patch, #12063 | |
Attached File | - | Added sys_sem_no_timeouts.sys.h.patch, #12064 | |||
Attached File | - | Added sys_sem_no_timeouts.sys.c.patch, #12065 | |||
2007-02-28 | dpotapov | Attached File | - | Added timeouts.expires.patch, #12061 |
Powered by Savane 3.13-d3ae.
Corresponding source code
OK, checked that in and closing as fixed.