Tcl Source Code

CopyData() has a bug in its management of the number
of bytes copied.  After reading and writing some bytes,
it should update the byte count.  However, before it
updates the byte count, it manages some fileevent
handlers in the case that the writing operation has
gone into the background.  In that circumstance, the
copy loop is exited without an update to the byte count,
leading to all the nastiness reported in Bug 118203.

The fix is simple:  Move the byte count update earlier
in the loop where it properly belongs.

The patch also fixes an error in the file event management,
where the wrong file event handler was being deleted.

Patch Code - Modified - New Version

On my Solaris 2.7 system, a 'make test' on the
contents of the CVS HEAD branch reports 0 tests failed
both before and after this patch is applied.

Committing to CVS HEAD...

Thanks for the explanations Don. If this change with the filehandlers doesn't break any of the existing tests I would accept the patch as it is.

The change from outChan to inChan is a fix unrelated to
Bug 118203.  I'm happy to move it to a separate patch if it
delays acceptance of this patch.

In generic/tclIO.c (Revision 1.27), compare lines 7058-7065
to lines 7089-7096.  It looks to me like those sections are
intended to change which channel, inChan or outChan, the
copy operation is waiting on.  In lines 7058-7065, we know
that inChan is not ready, so if we're already waiting on
inChan (mask & TCL_READABLE), do nothing.  If we're not
waiting on inChan, then check if we're waiting on outChan
(mask & TCL_WRITABLE).  If so, cancel that wait, and start
waiting on inChan, so when we exit the loop we're waiting
only on inChan.

To perform the analogous operation in lines 7089-7096, the
change included in this patch is necessary.  Then the logic
is that when outChan is not ready and we're not already
waiting for it, cancel any wait for inChan, and start
waiting for outChan.

My assumption is that the copy operation wants to be waiting
on only one of the channels at a time.  The fix implements
that assumption.  As currently written, the copy operation
can be waiting on both channels at the same time, and
CopyData() may be called many times by an uncancelled
readable fileevent when it can't do anything useful because
outChan is still not ready.

Why the change from outChan to inChan in the centre of the patch?

Patch Code - Modified - New Version

D'oh!  Correcting patch number...

OK, now please review.

Patch Code - Modified - New Version

Added patch id number to ChangeLog message.

Please review.