Ticket UUID: | 1ff0660e6e5f59639e50df751d4aaabb749f5583 | |||
Title: | Tcl_DriverInputProc is not called (again) | |||
Type: | Bug | Version: | 8.5.15 (trunk) | |
Submitter: | tombert | Created on: | 2014-07-30 07:03:03 | |
Subsystem: | 25. Channel System | Assigned To: | dgp | |
Priority: | 5 Medium | Severity: | Minor | |
Status: | Closed | Last Modified: | 2014-08-12 15:55:23 | |
Resolution: | Fixed | Closed By: | dgp | |
Closed on: | 2014-08-12 15:55:23 | |||
Description: |
The bug is verified on W7 (MinGW) and Ubuntu 12; The problem: The Tcl_DriverInputProc is called after a Tcl_NotifyChannel only once, succeeding calls to Tcl_NotifyChannel does not call the Tcl_DriverInputProc again. This was tested with the tclUdp package: https://sourceforge.net/projects/tcludp/ The bug could be bisect into following commit affecting file generic/tclIO.c: 1d526152bc4a40bbd9e801edc9c44e00fa107d5b is the first bad commit commit 1d526152bc4a40bbd9e801edc9c44e00fa107d5b Author: dgp <dgp> Date: Thu Mar 20 19:25:11 2014 +0000 Stop routine clearing of CHANNEL_EOF. Only clear when there's a reason (seek, eofchar change, ungets). Otherwise, once you hit EOF you stay there. Some additional infos and traces are here: https://groups.google.com/forum/#!topic/comp.lang.tcl/sRr678PXFfM | |||
User Comments: |
dgp added on 2014-08-12 15:55:23:
Put an accommodation in place for Tcl 8.5.16 so that patch release will not break deployed udp packages. Yummy carrot. Did not put that into Tcl 8.6.2. A patch fixing udp's bugs has been contributed. This is the stick to go along with the carrot to get "udp" to apply the patch and make a bug-fixed release. dgp added on 2014-08-11 20:17:57: Anyhow this confirms the diagnosis. udp expect to be able to [read] a channel that's @ eof and have that read attempt have the effect of clearing the EOF state and actually attempting the read machinery again. The recent revisions in tclIO.c had moved to the function where an EOF is permanent and all subsequent read attempts return empty string. Changing that would require something like a [seek] or an -eofchar change. It's a simple matter of putting the CHANNEL_EOF clear function back into CheckChannelError() to accommodate. dgp added on 2014-08-11 16:43:25: ....and then continuing bisecting over on the dgp-read-bytes branch: 4da5102566b1e10008363765e9516c2e575af201 2014-03-20 19:25:11 UTC Stop routine clearing of CHANNEL_EOF. ... dgp added on 2014-08-11 16:16:50: My own bisect shows the change in behavior on the 8.5 branch with checkin: ea760ac2443b173a981885e75406b828d8c30baa 2014-05-08 17:30:32 UTC dgp added on 2014-08-08 21:11:29: The original report here includes the result of a bisect. Can you please tell me how the demo script was used to distinguish success from failure in that bisect task? It's clear that parts of the demo script need an event loop. Is the testing done in a wish? ferrieux added on 2014-08-07 17:45:39: If you have the time and energy to do it, you might want to look at TIP 409 for an API that some of us think is better than the subverted channel model of TclUDP. tombert added on 2014-08-07 16:32:53: I have a bad feeling of introducing workarounds to tcl instead of fixing the package. Also I have a bad feeling of using packages that are not maintained. So the more I think about it the more I want to write my own udp package and having tcl itself kept "clean". Please do not stop releasing 8.5.16 only because of "me" ... dgp added on 2014-08-06 02:36:15: I cannot revise Tcl 8.5.16 to accommodate tclUdp without help. Either I have to get answers to my questions about what accommodations succeed or fail -- an interactive testing helper -- or I need a testing script that will fully tell me when I've succeeded in accommodation. "When this script runs again, things are acceptable" That demo script needs to be written for a child. I know essentially nothing about tclUdp and I'm not all that interested in learning. Don't say "it's wrong; fix it." because I don't know what's right. Give me a testing script that tells me when it is right. If I don't get this kind of help, I cannot avoid releasing Tcl 8.5.16 that breaks tclUDP. dgp added on 2014-07-30 18:06:12: While the tcludp folks consider that, lets' see what minimal change to the Tcl core can make things go again... Try disabling these lines http://core.tcl.tk/tcl/artifact/e6ca8641fa?ln=6549-6551 and see what happens. tombert added on 2014-07-30 17:53:20: Well and thx for helping out. I obtained the sources from CVS from sourceforge: cvs -d:pserver:[email protected]:/cvsroot/tcludp The bug I filed was quickly closed: https://sourceforge.net/p/tcludp/bugs/41/ But it seems that I need to reopen. Regards dgp added on 2014-07-30 17:43:41: Issue reported to tcludp project. https://sourceforge.net/p/tcludp/bugs/42/ dgp added on 2014-07-30 17:25:58: There is a bug in the tcludp routine udpInput(). When a Tcl_DriverInputProc() returns the value 0, the core I/O parts of Tcl take that to mean a signal of EOF on the channel. udpInput() is returning 0 in several places when it does not intend that meaning. if (statePtr->doread == 0) { statePtr->doread = 1; /* next time we want to behave normally */ *errorCode = EAGAIN; /* pretend that we would block */ UDPTRACE("Pretend we would block\n"); return 0; } That should be "return -1" instead. if (bufSize == 0) { return 0; } That's arguably also wrong, but harmless since the Tcl I/O core will never pass in a value of bufSize==0. if (packets == NULL) { UDPTRACE("packets is NULL\n"); return 0; } That should probably be "return -1". If this is an attempt to signal that the channel is blocked, then there should also be *errorCode = EAGAIN; and finally.... if (bytesRead > -1) { return bytesRead; } That might also return 0, whenever recvfrom() returns 0. If those circumstances correspond to EOF, then all is fine. Otherwise the above should be preceded by something like if (bytesRead == 0 && ThisIsNotEof()) { *errorCode = EAGAIN; return -1; } With those fixes, I think tclUdp will work again with current dev sources of Tcl and will also continue to work with all the deployed releases of Tcl. Is there an active upstream to send these changes to? dgp added on 2014-07-30 13:41:12: First of all, thank you for testing development builds of Tcl so we can catch these incompats before releasing them. My suspicion is that tclUdp is breaking an interface contract (admittedly one not spelled out all that clearly) and recent Tcl core changes have exposed that bug. To confirm, can you please point me to the latest tclUdp sources so I can reproduce the issue with my own testing? Either latest release, or latest dev state. |