| Ticket UUID: | 1582535 | |||
| Title: | Infinite loop in StateHandler | |||
| Type: | Patch | Version: | None | |
| Submitter: | hofkensg | Created on: | 2006-10-22 21:13:27 | |
| Subsystem: | ftp | Assigned To: | andreas_kupries | |
| Priority: | 5 Medium | Severity: | ||
| Status: | Closed | Last Modified: | 2006-10-24 03:05:45 | |
| Resolution: | Fixed | Closed By: | andreas_kupries | |
| Closed on: | 2006-10-23 19:33:30 | |||
| Description: |
Hello, I have discovered a series of events which cause the StateHandler proc to be called in an infinite loop. A possible scenario to trigger this is the following: - open a ftp connection and start a get - make sure the control connection times out because of network errors - try the ftp::Close proc and notice the "infinite number of loops" exception of tcl. The reason for this to happen is that the StateHandler passes through the [eof $ftp(CtrlSock)] branch, where it goes back to vwait in 'WaitComplete $s 0', without taking away the cause of the event. Meaning the ftp(CtrlSock) is still readable, and so the StateHandler is called again, and again, ... I have added a patchfile for this against revision 1.43 of ftp.tcl. Greetings, Guy | |||
| User Comments: |
hofkensg added on 2006-10-24 03:05:45:
Logged In: YES user_id=1625850 You are right about the no-op. I have tested revision 1.44 and it works fine. I will use the diff -u in the future. Thanks, Guy andreas_kupries added on 2006-10-24 02:33:30: Logged In: YES user_id=75003 I have now committed the patch without using the 'read CtrlSock'. If that causes the head (rev 1.44) to still fail with an infinite recursion please re-open this bug. Side note: Thank you providing a patch, and for attaching it to the report instead of inlining it. Despite this I have a small nit. Please use the command 'diff -u' the next time you create a patch for us. The '-u' switch causes 'diff' to generate a patch in the 'unified' format. This format generates larger patches, this however is offset by the fact that the result is much easier to understand when read by a human. In other words, it makes the reviewing of a patch easier for us maintainer. Thank you. andreas_kupries added on 2006-10-24 02:08:33: Logged In: YES user_id=75003 Reviewing the patch ... The move of the close/unset I understand. Takes the fileevent away. Unsetting ftp(state.data) I understand as well, after reading WaitComplete. It is necessary to prevent this command from doing a vwait without having an event source. However, why is there a 'read ftp(CtrlSock)' in the patch ? The Ctrlsock is already at eof, so this is a no-op. The close alone should be enough. hofkensg added on 2006-10-23 04:13:27: File Added - 199463: patch_ftp.tcl_1.43 | |||
Attachments:
- patch_ftp.tcl_1.43 [download] added by hofkensg on 2006-10-23 04:13:27. [details]