Tcl Source Code

View Ticket
Login
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.