Menu

#38 Fix for Bug 118203

closed
None
5
2001-01-30
2001-01-26
Don Porter
No

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.

Discussion

  • Don Porter

    Don Porter - 2001-01-26

    None

     
  • Don Porter

    Don Porter - 2001-01-26

    Added patch id number to ChangeLog message.

    Please review.

     
  • Don Porter

    Don Porter - 2001-01-26
    • assigned_to: nobody --> dgp
     
  • Don Porter

    Don Porter - 2001-01-26

    D'oh! Correcting patch number...

    OK, now please review.

     
  • Donal K. Fellows

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

     
  • Don Porter

    Don Porter - 2001-01-26

    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.

     
  • Andreas Kupries

    Andreas Kupries - 2001-01-30

    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.

     
  • Don Porter

    Don Porter - 2001-01-30

    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...

     
  • Don Porter

    Don Porter - 2001-01-30
    • status: open --> closed