From 91da54ded231d22a17875e77cd70306249f8f1a2 Mon Sep 17 00:00:00 2001 From: dgp Date: Fri, 8 Jun 2012 15:51:00 +0000 Subject: Work in progress fixing 3532959 --- generic/tclBasic.c | 3 ++- generic/tclProc.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/generic/tclBasic.c b/generic/tclBasic.c index 954b2b3..1d289f2 100644 --- a/generic/tclBasic.c +++ b/generic/tclBasic.c @@ -1388,7 +1388,8 @@ DeleteInterpProc( hPtr != NULL; hPtr = Tcl_NextHashEntry(&hSearch)) { CmdFrame *cfPtr = Tcl_GetHashValue(hPtr); - + Proc *procPtr = (Proc *) Tcl_GetHashKey(iPtr->linePBodyPtr, hPtr); + procPtr->iPtr = NULL; if (cfPtr->type == TCL_LOCATION_SOURCE) { Tcl_DecrRefCount(cfPtr->data.eval.path); } diff --git a/generic/tclProc.c b/generic/tclProc.c index 43e88c6..325506b 100644 --- a/generic/tclProc.c +++ b/generic/tclProc.c @@ -2198,7 +2198,7 @@ TclProcCleanupProc( * the same ProcPtr is overwritten with a new CmdFrame. */ - if (!iPtr) { + if (iPtr == NULL || iPtr->linePBodyPtr == NULL) { return; } -- cgit v0.12 From 66b97015952d286c20fdcb921b4b4158fbf876ca Mon Sep 17 00:00:00 2001 From: dgp Date: Sun, 10 Jun 2012 23:34:45 +0000 Subject: 3532959 Arrange for every lambda to place an entry in the linePBodyPtr hash table. Then the two teardowns of data in that table synchronize so that the first to run signals the other not to operate. Test proc-7.4 in a mem debug build of Tcl will detect Bug 3532959 by crashing. --- generic/tclBasic.c | 10 ++++++---- generic/tclProc.c | 22 +++++++++++++--------- tests/proc.test | 8 ++++++++ 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/generic/tclBasic.c b/generic/tclBasic.c index 1d289f2..fa13b50 100644 --- a/generic/tclBasic.c +++ b/generic/tclBasic.c @@ -1390,11 +1390,13 @@ DeleteInterpProc( CmdFrame *cfPtr = Tcl_GetHashValue(hPtr); Proc *procPtr = (Proc *) Tcl_GetHashKey(iPtr->linePBodyPtr, hPtr); procPtr->iPtr = NULL; - if (cfPtr->type == TCL_LOCATION_SOURCE) { - Tcl_DecrRefCount(cfPtr->data.eval.path); + if (cfPtr) { + if (cfPtr->type == TCL_LOCATION_SOURCE) { + Tcl_DecrRefCount(cfPtr->data.eval.path); + } + ckfree((char *) cfPtr->line); + ckfree((char *) cfPtr); } - ckfree((char *) cfPtr->line); - ckfree((char *) cfPtr); Tcl_DeleteHashEntry(hPtr); } Tcl_DeleteHashTable(iPtr->linePBodyPtr); diff --git a/generic/tclProc.c b/generic/tclProc.c index 325506b..7a93dbf 100644 --- a/generic/tclProc.c +++ b/generic/tclProc.c @@ -2198,7 +2198,7 @@ TclProcCleanupProc( * the same ProcPtr is overwritten with a new CmdFrame. */ - if (iPtr == NULL || iPtr->linePBodyPtr == NULL) { + if (iPtr == NULL) { return; } @@ -2209,13 +2209,15 @@ TclProcCleanupProc( cfPtr = (CmdFrame *) Tcl_GetHashValue(hePtr); - if (cfPtr->type == TCL_LOCATION_SOURCE) { - Tcl_DecrRefCount(cfPtr->data.eval.path); - cfPtr->data.eval.path = NULL; + if (cfPtr) { + if (cfPtr->type == TCL_LOCATION_SOURCE) { + Tcl_DecrRefCount(cfPtr->data.eval.path); + cfPtr->data.eval.path = NULL; + } + ckfree((char *) cfPtr->line); + cfPtr->line = NULL; + ckfree((char *) cfPtr); } - ckfree((char *) cfPtr->line); - cfPtr->line = NULL; - ckfree((char *) cfPtr); Tcl_DeleteHashEntry(hePtr); } @@ -2447,7 +2449,7 @@ SetLambdaFromAny( Interp *iPtr = (Interp *) interp; char *name; Tcl_Obj *argsPtr, *bodyPtr, *nsObjPtr, **objv, *errPtr; - int objc, result; + int isNew, objc, result; Proc *procPtr; if (interp == NULL) { @@ -2512,6 +2514,8 @@ SetLambdaFromAny( * common elements into a single function. */ + Tcl_SetHashValue(Tcl_CreateHashEntry(iPtr->linePBodyPtr, (char *) procPtr, + &isNew), NULL); if (iPtr->cmdFramePtr) { CmdFrame *contextPtr; @@ -2544,7 +2548,7 @@ SetLambdaFromAny( if (contextPtr->line && (contextPtr->nline >= 2) && (contextPtr->line[1] >= 0)) { - int isNew, buf[2]; + int buf[2]; CmdFrame *cfPtr = (CmdFrame *) ckalloc(sizeof(CmdFrame)); /* diff --git a/tests/proc.test b/tests/proc.test index 5673caa..c0f80e3 100644 --- a/tests/proc.test +++ b/tests/proc.test @@ -391,6 +391,14 @@ test proc-7.3 {Returning loop exception from redefined cmd: Bug 729692} { set res } {0 4} +test proc-7.4 {Proc struct outlives its interp: Bug 3532959} { + set lambda x + lappend lambda {set a 1} + interp create slave + slave eval [list apply $lambda foo] + interp delete slave + unset lambda +} {} # cleanup -- cgit v0.12 From b755b8b9f87b9bd65ad904bf18c4024d4ea91d14 Mon Sep 17 00:00:00 2001 From: dgp Date: Mon, 11 Jun 2012 12:21:30 +0000 Subject: Revised so that we avoid hashing twice. --- generic/tclProc.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/generic/tclProc.c b/generic/tclProc.c index 7a93dbf..2c6d300 100644 --- a/generic/tclProc.c +++ b/generic/tclProc.c @@ -2450,6 +2450,7 @@ SetLambdaFromAny( char *name; Tcl_Obj *argsPtr, *bodyPtr, *nsObjPtr, **objv, *errPtr; int isNew, objc, result; + CmdFrame *cfPtr = NULL; Proc *procPtr; if (interp == NULL) { @@ -2514,8 +2515,6 @@ SetLambdaFromAny( * common elements into a single function. */ - Tcl_SetHashValue(Tcl_CreateHashEntry(iPtr->linePBodyPtr, (char *) procPtr, - &isNew), NULL); if (iPtr->cmdFramePtr) { CmdFrame *contextPtr; @@ -2549,13 +2548,13 @@ SetLambdaFromAny( if (contextPtr->line && (contextPtr->nline >= 2) && (contextPtr->line[1] >= 0)) { int buf[2]; - CmdFrame *cfPtr = (CmdFrame *) ckalloc(sizeof(CmdFrame)); /* * Move from approximation (line of list cmd word) to actual * location (line of 2nd list element). */ + cfPtr = (CmdFrame *) ckalloc(sizeof(CmdFrame)); TclListLines(objPtr, contextPtr->line[1], 2, buf, NULL); cfPtr->level = -1; @@ -2571,9 +2570,6 @@ SetLambdaFromAny( cfPtr->cmd.str.cmd = NULL; cfPtr->cmd.str.len = 0; - - Tcl_SetHashValue(Tcl_CreateHashEntry(iPtr->linePBodyPtr, - (char *) procPtr, &isNew), cfPtr); } /* @@ -2585,6 +2581,8 @@ SetLambdaFromAny( } TclStackFree(interp, contextPtr); } + Tcl_SetHashValue(Tcl_CreateHashEntry(iPtr->linePBodyPtr, (char *) procPtr, + &isNew), cfPtr); /* * Set the namespace for this lambda: given by objv[2] understood as a -- cgit v0.12