From f3f5448b0aa89dfc3c913d3f2e43e34e1c0c106c 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 ac5377745066c2cc9fdc1d30ff2d449e2bb5b6d4 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 2c0667d7b04e34fa929ccc4758a19af166cf4206 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