Tcl Source Code

View Ticket
Login
Ticket UUID: 504642
Title: Problem in gets w/custom channels
Type: Bug Version: obsolete: 8.3.4
Submitter: bgriffin Created on: 2002-01-16 22:48:26
Subsystem: 24. Channel Commands Assigned To: andreas_kupries
Priority: 6 Severity:
Status: Closed Last Modified: 2002-01-18 03:38:02
Resolution: Fixed Closed By: andreas_kupries
    Closed on: 2002-01-17 20:38:02
Description:
    * Tcl Version: 8.3.4
    * OS Platform and Version:  Windows NT 4.0
    * Problem Behavior:

"gets stdin" crashes

    * Expected Behavior
Shouldn't crash...

    * Concise Code Sample (if applicable)

Sorry, but I don't have time to put together a concise
testcase, but I'll describe the problem as I understand
it and include the patch I made to fix it.

For our Windows application I've defined a console very
similar to the builtin Tk console.  This console
supports both stdout and stdin.  These console channels
 execute Tcl code (similar to TkConsolePrint) to
perform the I/O operation.

The code in Tcl_GetsObjCmd() obtains the ResultObj,
then passes this Obj to Tcl_GetsObj().  Tcl_GetsObj
then extracts the bytes pointer from the Obj (line
3104) and holds onto it until later.

Then the channel Input Proc is called and returns the
requested data. (Uh Oh!)

Next, some arithmetic is done between the Obj->bytes
pointer and the saved pointer (from before the call to
the Input Proc) and at this point bad things happen!

The problem is that this Obj is not reference counted
and is really owned by the interp.  Down inside the
channel Input Proc while executing some Tcl code, the
interp decides to free the Obj and replace it!  The
Gets code has no way to know this.

To fix this problem I change Tcl_GetsObjCmd to always
create a NewObj, forget using the interps Obj.  This
seems safer to me since you really have no clue what's
going to happen inside the Channel code.

Here's the changes to TcIOCmd.c core-8-3-4 (revision:
1.7.2.1):

cvs diff -p tclIOCmd.c
Index: tclIOCmd.c
===================================================================
RCS file: /cvsroot/tcl/tcl/generic/tclIOCmd.c,v
retrieving revision 1.7.2.1
diff -p -r1.7.2.1 tclIOCmd.c
*** tclIOCmd.c  2001/08/06 22:24:11     1.7.2.1
--- tclIOCmd.c  2002/01/16 22:05:57
*************** Tcl_GetsObjCmd(dummy, interp, objc, objv
*** 228,243 ****
          return TCL_ERROR;
      }
  
!     if (objc == 3) {
!       /*
!        * Variable gets line, interp get bytecount.
!        */
! 
!       linePtr = Tcl_NewObj();
!     }
!     else {
!       linePtr = Tcl_GetObjResult(interp);
!     }
  
      lineLen = Tcl_GetsObj(chan, linePtr);
      if (lineLen < 0) {
--- 228,234 ----
          return TCL_ERROR;
      }
  
!     linePtr = Tcl_NewObj();
  
      lineLen = Tcl_GetsObj(chan, linePtr);
      if (lineLen < 0) {
*************** Tcl_GetsObjCmd(dummy, interp, objc, objv
*** 261,266 ****
--- 252,259 ----
        resultPtr = Tcl_GetObjResult(interp);
        Tcl_SetIntObj(resultPtr, lineLen);
          return TCL_OK;
+     } else {
+       Tcl_SetObjResult(interp, linePtr);
      }
      return TCL_OK;
  }
User Comments: andreas_kupries added on 2002-01-18 03:38:02:
Logged In: YES 
user_id=75003

Patch applied, compiled, tested. Thought first that it 
introduces a mem.leak, but it doesn't.

Committed to head and core-8-3-1-branch.

andreas_kupries added on 2002-01-18 02:47:53:
Logged In: YES 
user_id=75003

Thanks.

bgriffin added on 2002-01-18 02:37:56:

File Added - 16299: patch.txt

bgriffin added on 2002-01-18 02:37:55:
Logged In: YES 
user_id=22949

Attached patch.
-Brian

andreas_kupries added on 2002-01-17 23:46:12:
Logged In: YES 
user_id=75003

The save/restore is currently only required if the channel 
driver murks with the interp result. For example if it 
executes a tcl script as part of its work.

Anbd yes, 8.0 did not have the save/restore routines.

I am willing to give your patch a shot, but please attach 
it.

bgriffin added on 2002-01-17 23:38:41:
Logged In: YES 
user_id=22949

This channel code was written for Tcl 8.0. Save/Restore of
the interp's state is done using Tcl_DStringGetResult() and
Tcl_DStringResult().

It seems to me that any advantage of grabbing the interps
Obj in gets is outweighed by the risk of the channel code
mucking with the interp. (IMHO) 

I don't see any save/restore in Tk's ConsoleOutput or
TkConsolePrint routines, so what exactly govern's the
decision when and when not to save/restore? 

I'll update my channel code accordingly in any event.

Sorry about the attachment.

Thanks,
-Brian

andreas_kupries added on 2002-01-17 06:00:04:
Logged In: YES 
user_id=75003

A different workaround to this situation is used by the 
package Trf. It uses the calls "Tcl_SaveResult" 
and "Tcl_RestoreResult" to open a new scope for the result 
of callbacks into tcl code from a channel driver, thus 
preventing the existing result from getting destroyed.

andreas_kupries added on 2002-01-17 05:57:27:
Logged In: YES 
user_id=75003

Educational hint for the future: Please "attach" patches 
(see below 'check to upload & attach file'), do not place 
them into the description of the problem. If the latter is 
done the formatting important to 'patch' is be destroyed. 
this makes application of provided patches ... difficult.

Thanks.

Attachments: