Index: generic/tclIO.c ================================================================== --- generic/tclIO.c +++ generic/tclIO.c @@ -10,10 +10,11 @@ * * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. */ +#undef NDEBUG #include "tclInt.h" #include "tclIO.h" #include /* @@ -165,10 +166,11 @@ static ChannelBuffer * AllocChannelBuffer(int length); static void PreserveChannelBuffer(ChannelBuffer *bufPtr); static void ReleaseChannelBuffer(ChannelBuffer *bufPtr); static int IsShared(ChannelBuffer *bufPtr); static void ChannelTimerProc(ClientData clientData); +static int ChanRead(Channel *chanPtr, char *dst, int dstSize); static int CheckChannelErrors(ChannelState *statePtr, int direction); static int CheckForDeadChannel(Tcl_Interp *interp, ChannelState *statePtr); static void CheckForStdChannelsBeingClosed(Tcl_Channel chan); @@ -343,26 +345,82 @@ (((st)->csPtrW) && ((fl) & TCL_WRITABLE))) #define MAX_CHANNEL_BUFFER_SIZE (1024*1024) /* - * ChanRead, dropped here by a time traveler, see 8.6 + *--------------------------------------------------------------------------- + * + * ChanRead -- + * + * Read up to dstSize bytes using the inputProc of chanPtr, store + * them at dst, and return the number of bytes stored. + * + * Results: + * The return value of the driver inputProc, + * - number of bytes stored at dst, ot + * - -1 on error, with a Posix error code available to the + * caller by calling Tcl_GetErrno(). + * + * Side effects: + * The CHANNEL_BLOCKED and CHANNEL_EOF flags of the channel state are + * set as appropriate. + * On EOF, the inputEncodingFlags are set to perform ending operations + * on decoding. + * TODO - Is this really the right place for that? + * + *--------------------------------------------------------------------------- */ -static inline int +static int ChanRead( Channel *chanPtr, char *dst, - int dstSize, - int *errnoPtr) + int dstSize) { + int bytesRead, result; + + /* + * If the caller asked for zero bytes, we'd force the inputProc + * to return zero bytes, and then misinterpret that as EOF. + */ + assert(dstSize > 0); + + /* + * Each read op must set the blocked and eof states anew, not let + * the effect of prior reads leak through. + */ + ResetFlag(chanPtr->state, CHANNEL_BLOCKED | CHANNEL_EOF); if (WillRead(chanPtr) < 0) { - *errnoPtr = Tcl_GetErrno(); return -1; } - return chanPtr->typePtr->inputProc(chanPtr->instanceData, dst, dstSize, - errnoPtr); + bytesRead = chanPtr->typePtr->inputProc(chanPtr->instanceData, + dst, dstSize, &result); + + /* Stop any flag leakage through stacked channel levels */ + ResetFlag(chanPtr->state, CHANNEL_BLOCKED | CHANNEL_EOF); + if (bytesRead > 0) { + /* + * If we get a short read, signal up that we may be BLOCKED. + * We should avoid calling the driver because on some + * platforms we will block in the low level reading code even + * though the channel is set into nonblocking mode. + */ + + if (bytesRead < dstSize) { + SetFlag(chanPtr->state, CHANNEL_BLOCKED); + } + } else if (bytesRead == 0) { + SetFlag(chanPtr->state, CHANNEL_EOF); + chanPtr->state->inputEncodingFlags |= TCL_ENCODING_END; + } else if (bytesRead < 0) { + if ((result == EWOULDBLOCK) || (result == EAGAIN)) { + SetFlag(chanPtr->state, CHANNEL_BLOCKED); + result = EAGAIN; + } + Tcl_SetErrno(result); + } + return bytesRead; } static inline Tcl_WideInt ChanSeek( Channel *chanPtr, @@ -4119,10 +4177,11 @@ */ Tcl_SetObjLength(objPtr, oldLength); CommonGetsCleanup(chanPtr); copiedTotal = -1; + ResetFlag(statePtr, CHANNEL_BLOCKED); goto done; } goto gotEOL; } dst = dstEnd; @@ -4318,15 +4377,13 @@ * All channel buffers were exhausted and the caller still * hasn't seen EOL. Need to read more bytes from the channel * device. Side effect is to allocate another channel buffer. */ - if (GotFlag(statePtr, CHANNEL_BLOCKED)) { - if (GotFlag(statePtr, CHANNEL_NONBLOCKING)) { - goto restore; - } - ResetFlag(statePtr, CHANNEL_BLOCKED); + if (GotFlag(statePtr, CHANNEL_BLOCKED|CHANNEL_NONBLOCKING) + == (CHANNEL_BLOCKED|CHANNEL_NONBLOCKING)) { + goto restore; } if (GetInput(chanPtr) != 0) { goto restore; } bufPtr = statePtr->inQueueTail; @@ -4385,10 +4442,11 @@ */ byteArray = Tcl_SetByteArrayLength(objPtr, oldLength); CommonGetsCleanup(chanPtr); copiedTotal = -1; + ResetFlag(statePtr, CHANNEL_BLOCKED); goto done; } goto gotEOL; } @@ -4588,18 +4646,10 @@ * EOL. Need to read more bytes from the channel device. Side effect * is to allocate another channel buffer. */ read: - if (GotFlag(statePtr, CHANNEL_BLOCKED)) { - if (GotFlag(statePtr, CHANNEL_NONBLOCKING)) { - gsPtr->charsWrote = 0; - gsPtr->rawRead = 0; - return -1; - } - ResetFlag(statePtr, CHANNEL_BLOCKED); - } if (GetInput(chanPtr) != 0) { gsPtr->charsWrote = 0; gsPtr->rawRead = 0; return -1; } @@ -4683,13 +4733,19 @@ bufPtr->nextRemoved = bufPtr->nextAdded; } else { /* * There are no more cached raw bytes left. See if we can get - * some more. + * some more, but avoid blocking on a non-blocking channel. */ + if (GotFlag(statePtr, CHANNEL_NONBLOCKING|CHANNEL_BLOCKED) + == (CHANNEL_NONBLOCKING|CHANNEL_BLOCKED)) { + gsPtr->charsWrote = 0; + gsPtr->rawRead = 0; + return -1; + } goto read; } } else { if (nextPtr == NULL) { nextPtr = AllocChannelBuffer(statePtr->bufSize); @@ -4930,11 +4986,11 @@ int bytesToRead) /* Maximum number of bytes to read. */ { Channel *chanPtr = (Channel *) chan; ChannelState *statePtr = chanPtr->state; /* State info for channel */ - int nread, result, copied, copiedNow; + int nread, copied, copiedNow = INT_MAX; /* * The check below does too much because it will reject a call to this * function with a channel which is part of an 'fcopy'. But we have to * allow this here or else the chaining in the transformation drivers will @@ -4955,78 +5011,35 @@ * it. Go to the driver only if there is none (anymore) and the caller * requests more bytes. */ Tcl_Preserve(chanPtr); - for (copied = 0; copied < bytesToRead; copied += copiedNow) { - copiedNow = CopyBuffer(chanPtr, bufPtr + copied, - bytesToRead - copied); - if (copiedNow == 0) { - if (GotFlag(statePtr, CHANNEL_EOF)) { - goto done; - } - if (GotFlag(statePtr, CHANNEL_BLOCKED)) { - if (GotFlag(statePtr, CHANNEL_NONBLOCKING)) { - goto done; - } - ResetFlag(statePtr, CHANNEL_BLOCKED); - } - - /* - * Now go to the driver to get as much as is possible to - * fill the remaining request. Do all the error handling by - * ourselves. The code was stolen from 'GetInput' and - * slightly adapted (different return value here). - * - * The case of 'bytesToRead == 0' at this point cannot - * happen. - */ - - nread = ChanRead(chanPtr, bufPtr + copied, - bytesToRead - copied, &result); - - if (nread > 0) { - /* - * If we get a short read, signal up that we may be BLOCKED. - * We should avoid calling the driver because on some - * platforms we will block in the low level reading code even - * though the channel is set into nonblocking mode. - */ - - if (nread < (bytesToRead - copied)) { - SetFlag(statePtr, CHANNEL_BLOCKED); - } - } else if (nread == 0) { - SetFlag(statePtr, CHANNEL_EOF); - statePtr->inputEncodingFlags |= TCL_ENCODING_END; - - } else if (nread < 0) { - if ((result == EWOULDBLOCK) || (result == EAGAIN)) { - if (copied > 0) { - /* - * Information that was copied earlier has precedence - * over EAGAIN/WOULDBLOCK handling. - */ - - goto done; - } - - SetFlag(statePtr, CHANNEL_BLOCKED); - result = EAGAIN; - } - - Tcl_SetErrno(result); + for (copied = 0; bytesToRead > 0 && copiedNow > 0; + bufPtr+=copiedNow, bytesToRead-=copiedNow, copied+=copiedNow) { + copiedNow = CopyBuffer(chanPtr, bufPtr, bytesToRead); + } + + if (bytesToRead > 0) { + /* + * Now go to the driver to get as much as is possible to + * fill the remaining request. Since we're directly filling + * the caller's buffer, retain the blocked flag. + */ + + nread = ChanRead(chanPtr, bufPtr, bytesToRead); + if (nread < 0) { + if (!GotFlag(statePtr, CHANNEL_BLOCKED) || copied == 0) { copied = -1; - goto done; } - + } else { copied += nread; - goto done; + } + if (copied != 0) { + ResetFlag(statePtr, CHANNEL_EOF); } } - done: Tcl_Release(chanPtr); return copied; } /* @@ -5156,10 +5169,12 @@ TclGetString(objPtr); } } + /* Must clear the BLOCKED flag here since we check before reading */ + ResetFlag(statePtr, CHANNEL_BLOCKED); for (copied = 0; (unsigned) toRead > 0; ) { copiedNow = -1; if (statePtr->inQueueHead != NULL) { if (binaryMode) { copiedNow = ReadBytes(statePtr, objPtr, toRead); @@ -5186,52 +5201,55 @@ if (copiedNow < 0) { if (GotFlag(statePtr, CHANNEL_EOF)) { break; } - if (GotFlag(statePtr, CHANNEL_BLOCKED)) { - if (GotFlag(statePtr, CHANNEL_NONBLOCKING)) { - break; - } - ResetFlag(statePtr, CHANNEL_BLOCKED); + if (GotFlag(statePtr, CHANNEL_NONBLOCKING|CHANNEL_BLOCKED) + == (CHANNEL_NONBLOCKING|CHANNEL_BLOCKED)) { + break; } result = GetInput(chanPtr); if (chanPtr != statePtr->topChanPtr) { Tcl_Release(chanPtr); chanPtr = statePtr->topChanPtr; Tcl_Preserve(chanPtr); } if (result != 0) { - if (result == EAGAIN) { - break; + if (!GotFlag(statePtr, CHANNEL_BLOCKED)) { + copied = -1; } - copied = -1; - goto done; + break; } } else { copied += copiedNow; toRead -= copiedNow; } } - ResetFlag(statePtr, CHANNEL_BLOCKED); - - /* - * Update the notifier state so we don't block while there is still data - * in the buffers. - */ - - done: + /* + * Failure to fill a channel buffer may have left channel reporting + * a "blocked" state, but so long as we fulfilled the request here, + * the caller does not consider us blocked. + */ + if (toRead == 0) { + ResetFlag(statePtr, CHANNEL_BLOCKED); + } + /* * Regenerate the top channel, in case it was changed due to * self-modifying reflected transforms. */ if (chanPtr != statePtr->topChanPtr) { Tcl_Release(chanPtr); chanPtr = statePtr->topChanPtr; Tcl_Preserve(chanPtr); } + + /* + * Update the notifier state so we don't block while there is still data + * in the buffers. + */ UpdateInterest(chanPtr); Tcl_Release(chanPtr); return copied; } @@ -6134,36 +6152,19 @@ if (GotFlag(statePtr, CHANNEL_EOF)) { return 0; } PreserveChannelBuffer(bufPtr); - nread = ChanRead(chanPtr, InsertPoint(bufPtr), toRead, &result); - if (nread > 0) { + nread = ChanRead(chanPtr, InsertPoint(bufPtr), toRead); + + if (nread < 0) { + result = Tcl_GetErrno(); + } else { result = 0; bufPtr->nextAdded += nread; - - /* - * If we get a short read, signal up that we may be BLOCKED. We should - * avoid calling the driver because on some platforms we will block in - * the low level reading code even though the channel is set into - * nonblocking mode. - */ - - if (nread < toRead) { - SetFlag(statePtr, CHANNEL_BLOCKED); - } - } else if (nread == 0) { - result = 0; - SetFlag(statePtr, CHANNEL_EOF); - statePtr->inputEncodingFlags |= TCL_ENCODING_END; - } else if (nread < 0) { - if ((result == EWOULDBLOCK) || (result == EAGAIN)) { - SetFlag(statePtr, CHANNEL_BLOCKED); - result = EAGAIN; - } - Tcl_SetErrno(result); - } + } + ReleaseChannelBuffer(bufPtr); return result; } /* @@ -6647,16 +6648,11 @@ Tcl_SetErrno(EBUSY); return -1; } if (direction == TCL_READABLE) { - /* - * Clear the BLOCKED bit. We want to discover this condition - * anew in each operation. - */ - - ResetFlag(statePtr, CHANNEL_BLOCKED | CHANNEL_NEED_MORE_DATA); + ResetFlag(statePtr, CHANNEL_NEED_MORE_DATA); } return 0; } @@ -8829,11 +8825,10 @@ /* If there is no full buffer, attempt to create and/or fill one. */ while (bufPtr == NULL || !IsBufferFull(bufPtr)) { int code; - ResetFlag(statePtr, CHANNEL_BLOCKED); moreData: code = GetInput(chanPtr); bufPtr = statePtr->inQueueHead; assert (bufPtr != NULL); @@ -8933,15 +8928,18 @@ statePtr->inQueueTail = NULL; } RecycleBuffer(statePtr, bufPtr, 0); } - if (GotFlag(statePtr, CHANNEL_NONBLOCKING) - && GotFlag(statePtr, CHANNEL_BLOCKED)) { + if (GotFlag(statePtr, CHANNEL_NONBLOCKING|CHANNEL_BLOCKED) + == (CHANNEL_NONBLOCKING|CHANNEL_BLOCKED)) { break; } } + if (bytesToRead == 0) { + ResetFlag(statePtr, CHANNEL_BLOCKED); + } Tcl_Release(chanPtr); return (int)(p - dst); } Index: tests/io.test ================================================================== --- tests/io.test +++ tests/io.test @@ -6703,15 +6703,27 @@ set f1 [open $thisScript] set f2 [open $path(test1) w] fconfigure $f1 -translation lf -blocking 0 fconfigure $f2 -translation cr -blocking 0 fcopy $f1 $f2 -size 40 - set result [list [fconfigure $f1 -blocking] [fconfigure $f2 -blocking]] + set result [list [fblocked $f1] [fconfigure $f1 -blocking] [fconfigure $f2 -blocking]] + close $f1 + close $f2 + lappend result [file size $path(test1)] +} {0 0 0 40} +test io-52.4.1 {TclCopyChannel} {fcopy} { + file delete $path(test1) + set f1 [open $thisScript] + set f2 [open $path(test1) w] + fconfigure $f1 -translation lf -blocking 0 -buffersize 10000000 + fconfigure $f2 -translation cr -blocking 0 + fcopy $f1 $f2 -size 40 + set result [list [fblocked $f1] [fconfigure $f1 -blocking] [fconfigure $f2 -blocking]] close $f1 close $f2 lappend result [file size $path(test1)] -} {0 0 40} +} {0 0 0 40} test io-52.5 {TclCopyChannel, all} {fcopy} { file delete $path(test1) set f1 [open $thisScript] set f2 [open $path(test1) w] fconfigure $f1 -translation lf -blocking 0