summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordgp <dgp@users.sourceforge.net>2014-05-29 15:04:52 (GMT)
committerdgp <dgp@users.sourceforge.net>2014-05-29 15:04:52 (GMT)
commita317bfbe0735d86682ee279cd1359576169bb8db (patch)
treed951445f494d1c37abde27c5ed76ca13e2802783
parent1d5af978995371afaa672cebd55f527be43abe94 (diff)
parentd344208a62df4671dac9a66fa4589979bc68d7bb (diff)
downloadtcl-a317bfbe0735d86682ee279cd1359576169bb8db.zip
tcl-a317bfbe0735d86682ee279cd1359576169bb8db.tar.gz
tcl-a317bfbe0735d86682ee279cd1359576169bb8db.tar.bz2
Refinements of FlushChannel() and its callers. Notably includes removal of the flag BUFFER_READY.
-rw-r--r--generic/tclIO.c172
-rw-r--r--generic/tclIO.h5
-rw-r--r--tests/io.test14
3 files changed, 73 insertions, 118 deletions
diff --git a/generic/tclIO.c b/generic/tclIO.c
index 0c13bc0..d356e11 100644
--- a/generic/tclIO.c
+++ b/generic/tclIO.c
@@ -288,7 +288,7 @@ static int WillRead(Channel *chanPtr);
#define IsBufferEmpty(bufPtr) ((bufPtr)->nextAdded == (bufPtr)->nextRemoved)
-#define IsBufferFull(bufPtr) ((bufPtr)->nextAdded >= (bufPtr)->bufLength)
+#define IsBufferFull(bufPtr) ((bufPtr) && (bufPtr)->nextAdded >= (bufPtr)->bufLength)
#define IsBufferOverflowing(bufPtr) ((bufPtr)->nextAdded>(bufPtr)->bufLength)
@@ -1238,15 +1238,6 @@ Tcl_UnregisterChannel(
*/
if (statePtr->refCount <= 0) {
- /*
- * Ensure that if there is another buffer, it gets flushed whether or
- * not we are doing a background flush.
- */
-
- if ((statePtr->curOutPtr != NULL) &&
- IsBufferReady(statePtr->curOutPtr)) {
- SetFlag(statePtr, BUFFER_READY);
- }
Tcl_Preserve(statePtr);
if (!GotFlag(statePtr, BG_FLUSH_SCHEDULED)) {
/*
@@ -2579,8 +2570,6 @@ FlushChannel(
ChannelState *statePtr = chanPtr->state;
/* State of the channel stack. */
ChannelBuffer *bufPtr; /* Iterates over buffered output queue. */
- int toWrite; /* Amount of output data in current buffer
- * available to be written. */
int written; /* Amount of output data actually written in
* current round. */
int errorCode = 0; /* Stores POSIX error codes from channel
@@ -2600,64 +2589,58 @@ FlushChannel(
}
/*
- * Loop over the queued buffers and attempt to flush as much as possible
- * of the queued output to the channel.
+ * Should we shift the current output buffer over to the output queue?
+ * First check that there are bytes in it. If so then...
+ * If the output queue is empty, then yes, trusting the caller called
+ * us only when written bytes ought to be flushed.
+ * If the current output buffer is full, then yes, so we can meet
+ * the post-condition that on a successful return to caller we've
+ * left space in the current output buffer for more writing (the flush
+ * call was to make new room).
+ * Otherwise, no. Keep the current output buffer where it is so more
+ * can be written to it, possibly filling it, to promote more efficient
+ * buffer usage.
*/
- Tcl_Preserve(chanPtr);
- while (1) {
- /*
- * If the queue is empty and there is a ready current buffer, OR if
- * the current buffer is full, then move the current buffer to the
- * queue.
- */
-
- if (((statePtr->curOutPtr != NULL) &&
- IsBufferFull(statePtr->curOutPtr))
- || (GotFlag(statePtr, BUFFER_READY) &&
- (statePtr->outQueueHead == NULL))) {
- ResetFlag(statePtr, BUFFER_READY);
- statePtr->curOutPtr->nextPtr = NULL;
- if (statePtr->outQueueHead == NULL) {
- statePtr->outQueueHead = statePtr->curOutPtr;
- } else {
- statePtr->outQueueTail->nextPtr = statePtr->curOutPtr;
- }
- statePtr->outQueueTail = statePtr->curOutPtr;
- statePtr->curOutPtr = NULL;
+ bufPtr = statePtr->curOutPtr;
+ if (bufPtr && BytesLeft(bufPtr) && /* Keep empties off queue */
+ (statePtr->outQueueHead == NULL || IsBufferFull(bufPtr))) {
+ if (statePtr->outQueueHead == NULL) {
+ statePtr->outQueueHead = bufPtr;
+ } else {
+ statePtr->outQueueTail->nextPtr = bufPtr;
}
- bufPtr = statePtr->outQueueHead;
+ statePtr->outQueueTail = bufPtr;
+ statePtr->curOutPtr = NULL;
+ }
- /*
- * If we are not being called from an async flush and an async flush
- * is active, we just return without producing any output.
- */
+ assert(!IsBufferFull(statePtr->curOutPtr));
- if (!calledFromAsyncFlush && GotFlag(statePtr, BG_FLUSH_SCHEDULED)) {
- errorCode = 0;
- goto done;
- }
+ /*
+ * If we are not being called from an async flush and an async flush
+ * is active, we just return without producing any output.
+ */
- /*
- * If the output queue is still empty, break out of the while loop.
- */
+ if (!calledFromAsyncFlush && GotFlag(statePtr, BG_FLUSH_SCHEDULED)) {
+ return 0;
+ }
- if (bufPtr == NULL) {
- break; /* Out of the "while (1)". */
- }
+ /*
+ * Loop over the queued buffers and attempt to flush as much as possible
+ * of the queued output to the channel.
+ */
+
+ Tcl_Preserve(chanPtr);
+ while (statePtr->outQueueHead) {
+ bufPtr = statePtr->outQueueHead;
/*
* Produce the output on the channel.
*/
PreserveChannelBuffer(bufPtr);
- toWrite = BytesLeft(bufPtr);
- if (toWrite == 0) {
- written = 0;
- } else {
- written = ChanWrite(chanPtr, RemovePoint(bufPtr), toWrite,
- &errorCode);
- }
+ written = ChanWrite(chanPtr, RemovePoint(bufPtr), BytesLeft(bufPtr),
+ &errorCode);
/*
* If the write failed completely attempt to start the asynchronous
@@ -2757,8 +2740,10 @@ FlushChannel(
DiscardOutputQueued(statePtr);
ReleaseChannelBuffer(bufPtr);
- continue;
+ break;
} else {
+ /* TODO: Consider detecting and reacting to short writes
+ * on blocking channels. Ought not happen. See iocmd-24.2. */
wroteSome = 1;
}
@@ -2776,7 +2761,7 @@ FlushChannel(
RecycleBuffer(statePtr, bufPtr, 0);
}
ReleaseChannelBuffer(bufPtr);
- } /* Closes "while (1)". */
+ } /* Closes "while". */
/*
* If we wrote some data while flushing in the background, we are done.
@@ -2791,6 +2776,12 @@ FlushChannel(
} else if (statePtr->outQueueHead == NULL) {
ResetFlag(statePtr, BG_FLUSH_SCHEDULED);
ChanWatch(chanPtr, statePtr->interestMask);
+ } else {
+ /* TODO: If code reaches this point, it means a writable
+ * event is being handled on the channel, but the channel
+ * could not in fact be written to. This ought not happen,
+ * but Unix pipes appear to act this way (see io-53.4).
+ * Also can imagine broken reflected channels. */
}
}
@@ -3328,14 +3319,6 @@ Tcl_Close(
ResetFlag(statePtr, CHANNEL_INCLOSE);
/*
- * Ensure that the last output buffer will be flushed.
- */
-
- if ((statePtr->curOutPtr != NULL) && IsBufferReady(statePtr->curOutPtr)) {
- SetFlag(statePtr, BUFFER_READY);
- }
-
- /*
* If this channel supports it, close the read side, since we don't need
* it anymore and this will help avoid deadlocks on some channel types.
*/
@@ -3503,10 +3486,6 @@ Tcl_CloseEx(
return CloseChannelPart(interp, chanPtr, 0, flags);
} else if (flags & TCL_CLOSE_WRITE) {
- if ((statePtr->curOutPtr != NULL) &&
- IsBufferReady(statePtr->curOutPtr)) {
- SetFlag(statePtr, BUFFER_READY);
- }
Tcl_Preserve(statePtr);
if (!GotFlag(statePtr, BG_FLUSH_SCHEDULED)) {
/*
@@ -3568,14 +3547,6 @@ CloseWrite(
int result = 0;
/*
- * Ensure that the last output buffer will be flushed.
- */
-
- if ((statePtr->curOutPtr != NULL) && IsBufferReady(statePtr->curOutPtr)) {
- SetFlag(statePtr, BUFFER_READY);
- }
-
- /*
* The call to FlushChannel will flush any queued output and invoke the
* close function of the channel driver, or it will set up the channel to
* be flushed and closed asynchronously.
@@ -4095,10 +4066,17 @@ WillRead(
}
if ((chanPtr->typePtr->seekProc != NULL)
&& (Tcl_OutputBuffered((Tcl_Channel) chanPtr) > 0)) {
- if ((chanPtr->state->curOutPtr != NULL)
- && IsBufferReady(chanPtr->state->curOutPtr)) {
- SetFlag(chanPtr->state, BUFFER_READY);
- }
+
+ /*
+ * CAVEAT - The assumption here is that FlushChannel() will
+ * push out the bytes of any writes that are in progress.
+ * Since this is a seekable channel, we assume it is not one
+ * that can block and force bg flushing. Channels we know that
+ * can do that -- sockets, pipes -- are not seekable. If the
+ * assumption is wrong, more drastic measures may be required here
+ * like temporarily setting the channel into blocking mode.
+ */
+
if (FlushChannel(NULL, chanPtr, 0) != 0) {
return -1;
}
@@ -4280,7 +4258,6 @@ Write(
}
if ((flushed < total) && (GotFlag(statePtr, CHANNEL_UNBUFFERED) ||
(needNlFlush && GotFlag(statePtr, CHANNEL_LINEBUFFERED)))) {
- SetFlag(statePtr, BUFFER_READY);
if (FlushChannel(NULL, chanPtr, 0) != 0) {
return -1;
}
@@ -6400,14 +6377,6 @@ Tcl_Flush(
return -1;
}
- /*
- * Force current output buffer to be output also.
- */
-
- if ((statePtr->curOutPtr != NULL) && IsBufferReady(statePtr->curOutPtr)) {
- SetFlag(statePtr, BUFFER_READY);
- }
-
result = FlushChannel(NULL, chanPtr, 0);
if (result != 0) {
return TCL_ERROR;
@@ -6547,9 +6516,8 @@ GetInput(
*/
bufPtr = statePtr->inQueueTail;
- if ((bufPtr != NULL) && !IsBufferFull(bufPtr)) {
- toRead = SpaceLeft(bufPtr);
- } else {
+
+ if ((bufPtr == NULL) || IsBufferFull(bufPtr)) {
bufPtr = statePtr->saveInBufPtr;
statePtr->saveInBufPtr = NULL;
@@ -6579,6 +6547,8 @@ GetInput(
statePtr->inQueueTail->nextPtr = bufPtr;
}
statePtr->inQueueTail = bufPtr;
+ } else {
+ toRead = SpaceLeft(bufPtr);
}
PreserveChannelBuffer(bufPtr);
@@ -6720,15 +6690,6 @@ Tcl_Seek(
}
/*
- * If there is data buffered in statePtr->curOutPtr then mark the channel
- * as ready to flush before invoking FlushChannel.
- */
-
- if ((statePtr->curOutPtr != NULL) && IsBufferReady(statePtr->curOutPtr)) {
- SetFlag(statePtr, BUFFER_READY);
- }
-
- /*
* If the flush fails we cannot recover the original position. In that
* case the seek is not attempted because we do not know where the access
* position is - instead we return the error. FlushChannel has already
@@ -9244,7 +9205,7 @@ DoRead(
/* If there is no full buffer, attempt to create and/or fill one. */
- while (bufPtr == NULL || !IsBufferFull(bufPtr)) {
+ while (!IsBufferFull(bufPtr)) {
int code;
moreData:
@@ -10794,7 +10755,6 @@ DumpFlags(
ChanFlag('n', CHANNEL_NONBLOCKING);
ChanFlag('l', CHANNEL_LINEBUFFERED);
ChanFlag('u', CHANNEL_UNBUFFERED);
- ChanFlag('R', BUFFER_READY);
ChanFlag('F', BG_FLUSH_SCHEDULED);
ChanFlag('c', CHANNEL_CLOSED);
ChanFlag('E', CHANNEL_EOF);
diff --git a/generic/tclIO.h b/generic/tclIO.h
index 4133ee0..097cd61 100644
--- a/generic/tclIO.h
+++ b/generic/tclIO.h
@@ -227,11 +227,6 @@ typedef struct ChannelState {
* flushed after every newline. */
#define CHANNEL_UNBUFFERED (1<<5) /* Output to the channel must always
* be flushed immediately. */
-#define BUFFER_READY (1<<6) /* Current output buffer (the
- * curOutPtr field in the channel
- * structure) should be output as soon
- * as possible even though it may not
- * be full. */
#define BG_FLUSH_SCHEDULED (1<<7) /* A background flush of the queued
* output buffers has been
* scheduled. */
diff --git a/tests/io.test b/tests/io.test
index adcc3da..023af06 100644
--- a/tests/io.test
+++ b/tests/io.test
@@ -2810,7 +2810,7 @@ test io-29.34 {Tcl_Close, async flush on close, using sockets} {socket tempNotMa
variable x running
set l abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz
proc writelots {s l} {
- for {set i 0} {$i < 2000} {incr i} {
+ for {set i 0} {$i < 9000} {incr i} {
puts $s $l
}
}
@@ -2841,7 +2841,7 @@ test io-29.34 {Tcl_Close, async flush on close, using sockets} {socket tempNotMa
close $ss
vwait [namespace which -variable x]
set c
-} 2000
+} 9000
test io-29.35 {Tcl_Close vs fileevent vs multiple interpreters} {socket tempNotMac fileevent} {
# On Mac, this test screws up sockets such that subsequent tests using port 2828
# either cause errors or panic().
@@ -7144,17 +7144,17 @@ test io-53.4 {CopyData: background write overflow} {stdio unix openpipe fileeven
for {set x 0} {$x < 12} {incr x} {
append big $big
}
- file delete $path(test1)
+# file delete $path(test1)
file delete $path(pipe)
set f1 [open $path(pipe) w]
puts $f1 {
puts ready
fcopy stdin stdout -command { set x }
vwait x
- set f [open $path(test1) w]
- fconfigure $f -translation lf
- puts $f "done"
- close $f
+# set f [open $path(test1) w]
+# fconfigure $f -translation lf
+# puts $f "done"
+# close $f
}
close $f1
set f1 [open "|[list [interpreter] $path(pipe)]" r+]