From 79f72560e381793712dfb3e1962527bb3295805d Mon Sep 17 00:00:00 2001 From: andreas_kupries Date: Thu, 15 Jul 2004 20:46:16 +0000 Subject: * generic/tclIO.h (CHANNEL_INCLOSE): New flag. Set in * generic/tclIO.c (Tcl_UnregisterChannel): 'Tcl_Close' while the * generic/tclIO.c (Tcl_Close): close callbacks are run. Checked in 'Tcl_Close' and 'Tcl_Unregister' to prevent recursive call of 'close' in the close-callbacks. This is a possible error made by implementors of virtual filesystems based on 'tclvfs', thinking that they have to close the channel in the close handler for the filesystem. * generic/tclIO.c: * generic/tclIO.h: * Not reverting, but #ifdef'ing the changes from May 19, 2004 out of the core. This removes the ***POTENTIAL INCOMPATIBILITY*** for channel drivers it introduced. This has become possible due to Expect gaining a BlockModeProc and now handling blockingg and non-blocking modes correctly. Thus [SF Tcl Bug 943274] is still fixed if a recent enough version of Expect is used. * doc/CrtChannel.3: Added warning about usage of a channel without a BlockModeProc. FossilOrigin-Name: 91ff6467230152d7ee06113589b3a4be1c0e2b87 --- ChangeLog | 31 +++++++++++++++++++++++++++++-- doc/CrtChannel.3 | 10 +++++++++- generic/tclIO.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- generic/tclIO.h | 11 ++++++++++- 4 files changed, 96 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 70bd297..99ffef6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2004-07-14 Andreas Kupries + + * generic/tclIO.h (CHANNEL_INCLOSE): New flag. Set in + * generic/tclIO.c (Tcl_UnregisterChannel): 'Tcl_Close' while the + * generic/tclIO.c (Tcl_Close): close callbacks are + run. Checked in 'Tcl_Close' and 'Tcl_Unregister' to prevent + recursive call of 'close' in the close-callbacks. This is a + possible error made by implementors of virtual filesystems based + on 'tclvfs', thinking that they have to close the channel in the + close handler for the filesystem. + +2004-06-14 Andreas Kupries + + * generic/tclIO.c: + * generic/tclIO.h: + * Not reverting, but #ifdef'ing the changes from May 19, 2004 out + of the core. This removes the ***POTENTIAL INCOMPATIBILITY*** + for channel drivers it introduced. This has become possible due + to Expect gaining a BlockModeProc and now handling blockingg and + non-blocking modes correctly. Thus [SF Tcl Bug 943274] is still + fixed if a recent enough version of Expect is used. + + * doc/CrtChannel.3: Added warning about usage of a channel without + a BlockModeProc. + 2004-07-15 Andreas Kupries * generic/tclIOCmd.c (Tcl_PutsObjCmd): Added length check to the @@ -190,8 +215,8 @@ 2004-05-19 Andreas Kupries - * tclIO.c: Fixed [SF Tcl Bug 943274]. This is the same problem as - * tclIO.h: [SF Tcl Bug 462317], see ChangeLog entry + * generic/tclIO.c: Fixed [SF Tcl Bug 943274]. This is the same problem as + * generic/tclIO.h: [SF Tcl Bug 462317], see ChangeLog entry 2001-09-26. The fix done at that time is incomplete. It is possible to get around it if the actual read operation is defered and not executed in the event @@ -202,6 +227,8 @@ actual data waiting. The flag is cleared by a short or full read. + ***POTENTIAL INCOMPATIBILITY*** for channel drivers. + 2004-05-18 Kevin B. Kenny * compat/strftime.c (_fmt, ISO8601Week): diff --git a/doc/CrtChannel.3 b/doc/CrtChannel.3 index 477ef67..828b958 100644 --- a/doc/CrtChannel.3 +++ b/doc/CrtChannel.3 @@ -5,7 +5,7 @@ '\" See the file "license.terms" for information on usage and redistribution '\" of this file, and for a DISCLAIMER OF ALL WARRANTIES. '\" -'\" RCS: @(#) $Id: CrtChannel.3,v 1.16 2002/07/01 18:24:39 jenglish Exp $ +'\" RCS: @(#) $Id: CrtChannel.3,v 1.16.2.1 2004/07/15 20:46:17 andreas_kupries Exp $ .so man.macros .TH Tcl_CreateChannel 3 8.3 Tcl "Tcl Library Procedures" .BS @@ -405,6 +405,14 @@ behavior must be emulated in the channel driver. .PP This value can be retrieved with \fBTcl_ChannelBlockModeProc\fR, which returns a pointer to the function. +.PP +A channel driver \fBnot\fR supplying a \fIblockModeProc\fR has to be +very, very careful. It has to tell the generic layer exactly which +blocking mode is acceptable to it, and should this also document for +the user so that the blocking mode of the channel is not changed to an +inacceptable value. Any confusion here may lead the interpreter into a +(spurious and difficult to find) deadlock. + .SH "CLOSEPROC AND CLOSE2PROC" .PP diff --git a/generic/tclIO.c b/generic/tclIO.c index f1a3266..154bec0 100644 --- a/generic/tclIO.c +++ b/generic/tclIO.c @@ -10,7 +10,7 @@ * See the file "license.terms" for information on usage and redistribution * of this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tclIO.c,v 1.61.2.6 2004/05/19 19:23:58 andreas_kupries Exp $ + * RCS: @(#) $Id: tclIO.c,v 1.61.2.7 2004/07/15 20:46:18 andreas_kupries Exp $ */ #include "tclInt.h" @@ -772,6 +772,7 @@ Tcl_RegisterChannel(interp, chan) } Tcl_SetHashValue(hPtr, (ClientData) chanPtr); } + statePtr->refCount++; } @@ -808,6 +809,17 @@ Tcl_UnregisterChannel(interp, chan) { ChannelState *statePtr; /* State of the real channel. */ + statePtr = ((Channel *) chan)->state->bottomChanPtr->state; + + if (statePtr->flags & CHANNEL_INCLOSE) { + if (interp != (Tcl_Interp*) NULL) { + Tcl_AppendResult(interp, + "Illegal recursive call to close through close-handler of channel", + (char *) NULL); + } + return TCL_ERROR; + } + if (DetachChannel(interp, chan) != TCL_OK) { return TCL_OK; } @@ -2527,6 +2539,14 @@ Tcl_Close(interp, chan) if (statePtr->refCount > 0) { panic("called Tcl_Close on channel with refCount > 0"); } + + if (statePtr->flags & CHANNEL_INCLOSE) { + Tcl_AppendResult(interp, + "Illegal recursive call to close through close-handler of channel", + (char *) NULL); + return TCL_ERROR; + } + statePtr->flags |= CHANNEL_INCLOSE; /* * When the channel has an escape sequence driven encoding such as @@ -2551,6 +2571,8 @@ Tcl_Close(interp, chan) ckfree((char *) cbPtr); } + statePtr->flags &= ~CHANNEL_INCLOSE; + /* * Ensure that the last output buffer will be flushed. */ @@ -4225,6 +4247,7 @@ Tcl_ReadRaw(chan, bufPtr, bytesToRead) statePtr->flags &= (~(CHANNEL_BLOCKED)); } +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING /* [SF Tcl Bug 943274]. Better emulation of non-blocking * channels for channels without BlockModeProc, by keeping * track of true fileevents generated by the OS == Data @@ -4240,6 +4263,7 @@ Tcl_ReadRaw(chan, bufPtr, bytesToRead) nread = -1; result = EWOULDBLOCK; } else { +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ /* * Now go to the driver to get as much as is possible to * fill the remaining request. Do all the error handling @@ -4251,7 +4275,9 @@ Tcl_ReadRaw(chan, bufPtr, bytesToRead) nread = (chanPtr->typePtr->inputProc)(chanPtr->instanceData, bufPtr + copied, bytesToRead - copied, &result); +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING } +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ if (nread > 0) { /* * If we get a short read, signal up that we may be @@ -4265,11 +4291,13 @@ Tcl_ReadRaw(chan, bufPtr, bytesToRead) statePtr->flags |= CHANNEL_BLOCKED; } +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING if (nread <= (bytesToRead - copied)) { /* [SF Tcl Bug 943274] We have read the available * data, clear flag */ statePtr->flags &= ~CHANNEL_HAS_MORE_DATA; } +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ } else if (nread == 0) { statePtr->flags |= CHANNEL_EOF; statePtr->inputEncodingFlags |= TCL_ENCODING_END; @@ -5310,6 +5338,7 @@ GetInput(chanPtr) return 0; } +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING /* [SF Tcl Bug 943274]. Better emulation of non-blocking channels * for channels without BlockModeProc, by keeping track of true * fileevents generated by the OS == Data waiting and reading if @@ -5324,9 +5353,14 @@ GetInput(chanPtr) nread = -1; result = EWOULDBLOCK; } else { +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ + nread = (chanPtr->typePtr->inputProc)(chanPtr->instanceData, bufPtr->buf + bufPtr->nextAdded, toRead, &result); + +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING } +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ if (nread > 0) { bufPtr->nextAdded += nread; @@ -5342,11 +5376,13 @@ GetInput(chanPtr) statePtr->flags |= CHANNEL_BLOCKED; } +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING if (nread <= toRead) { /* [SF Tcl Bug 943274] We have read the available data, * clear flag */ statePtr->flags &= ~CHANNEL_HAS_MORE_DATA; } +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ } else if (nread == 0) { statePtr->flags |= CHANNEL_EOF; @@ -6740,6 +6776,7 @@ Tcl_NotifyChannel(channel, mask) Channel* upChanPtr; Tcl_ChannelType* upTypePtr; +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING /* [SF Tcl Bug 943274] * For a non-blocking channel without blockmodeproc we keep track * of actual input coming from the OS so that we can do a credible @@ -6753,6 +6790,7 @@ Tcl_NotifyChannel(channel, mask) statePtr->flags |= CHANNEL_HAS_MORE_DATA; } +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ /* * In contrast to the other API functions this procedure walks towards @@ -6993,6 +7031,7 @@ ChannelTimerProc(clientData) statePtr->timer = Tcl_CreateTimerHandler(0, ChannelTimerProc, (ClientData) chanPtr); +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING /* Set the TIMER flag to notify the higher levels that the * driver might have no data for us. We do this only if we are * in non-blocking mode and the driver has no BlockModeProc @@ -7004,10 +7043,15 @@ ChannelTimerProc(clientData) (Tcl_ChannelBlockModeProc(chanPtr->typePtr) == NULL)) { statePtr->flags |= CHANNEL_TIMER_FEV; } +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ + Tcl_Preserve((ClientData) statePtr); Tcl_NotifyChannel((Tcl_Channel)chanPtr, TCL_READABLE); +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING statePtr->flags &= ~CHANNEL_TIMER_FEV; +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ + Tcl_Release((ClientData) statePtr); } else { statePtr->timer = NULL; @@ -9239,8 +9283,11 @@ DumpFlags (str, flags) if (flags & INPUT_NEED_NL) {buf[i] = '*';} else {buf [i]='_';}; i++; if (flags & CHANNEL_DEAD) {buf[i] = 'D';} else {buf [i]='_';}; i++; if (flags & CHANNEL_RAW_MODE) {buf[i] = 'R';} else {buf [i]='_';}; i++; +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING if (flags & CHANNEL_TIMER_FEV) {buf[i] = 'T';} else {buf [i]='_';}; i++; if (flags & CHANNEL_HAS_MORE_DATA) {buf[i] = 'H';} else {buf [i]='_';}; i++; +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ + if (flags & CHANNEL_INCLOSE) {buf[i] = 'x';} else {buf [i]='_';}; i++; buf [i] ='\0'; fprintf (stderr,"%s: %s\n", str, buf); fflush(stderr); diff --git a/generic/tclIO.h b/generic/tclIO.h index 7f2c421..ea3c6e4 100644 --- a/generic/tclIO.h +++ b/generic/tclIO.h @@ -10,7 +10,7 @@ * See the file "license.terms" for information on usage and redistribution * of this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tclIO.h,v 1.5.4.1 2004/05/19 19:16:24 andreas_kupries Exp $ + * RCS: @(#) $Id: tclIO.h,v 1.5.4.2 2004/07/15 20:46:19 andreas_kupries Exp $ */ /* @@ -296,6 +296,7 @@ typedef struct ChannelState { * the state of the channel changes. */ #define CHANNEL_RAW_MODE (1<<16) /* When set, notes that the Raw API is * being used. */ +#ifdef TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING #define CHANNEL_TIMER_FEV (1<<17) /* When set the event we are * notified by is a fileevent * generated by a timer. We @@ -323,6 +324,14 @@ typedef struct ChannelState { * will be performed if the * flag is set. This will * reset the flag as well. */ +#endif /* TCL_IO_TRACK_OS_FOR_DRIVER_WITH_BAD_BLOCKING */ + +#define CHANNEL_INCLOSE (1<<19) /* Channel is currently being + * closed. Its structures are + * still live and usable, but + * it may not be closed again + * from within the close handler. + */ /* * For each channel handler registered in a call to Tcl_CreateChannelHandler, -- cgit v0.12