From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10408 invoked by alias); 13 Sep 2002 22:42:18 -0000 Mailing-List: contact insight-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: insight-owner@sources.redhat.com Received: (qmail 10401 invoked from network); 13 Sep 2002 22:42:18 -0000 Received: from unknown (HELO mta6.snfc21.pbi.net) (206.13.28.240) by sources.redhat.com with SMTP; 13 Sep 2002 22:42:18 -0000 Received: from modrick ([66.120.11.181]) by mta6.snfc21.pbi.net (iPlanet Messaging Server 5.1 (built May 7 2001)) with SMTP id <0H2E00FDEF2GGW@mta6.snfc21.pbi.net> for insight@sources.redhat.com; Fri, 13 Sep 2002 15:42:17 -0700 (PDT) Date: Fri, 13 Sep 2002 15:42:00 -0000 From: Mo DeJong Subject: Re: Request for patch review. In-reply-to: To: insight@sources.redhat.com Message-id: <20020913154256.063fc8a3.supermo@bayarea.net> Organization: House of Mirth MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit References: <20020912152213.2ef53ecd.supermo@bayarea.net> X-SW-Source: 2002-q3/txt/msg00161.txt.bz2 On Fri, 13 Sep 2002 07:46:19 -0700 (PDT) Keith Seitz wrote: > On Thu, 12 Sep 2002, Mo DeJong wrote: > > > I posted this patch some time ago, but it seems to have > > fallen through the cracks. > > Definitely fallen through the cracks, out the other side, and back again. > > Committed. > > Keith Thanks much. I do have just one more that also fell through a crack. As I mentioned earlier, this code change was already made in the CVS for the net release of Itk and it includes test cases. cheers Mo 2002-09-13 Mo DeJong * itk/library/Toplevel.itk (destructor): * itk/library/Widget.itk (destructor): Protect against case where one external component destroy destroys another componet widget. * itk/tests/toplevel.test: * itk/tests/widget.test: Test for bug where one external component deletes another during the itk destructor. Index: itcl/itk/library/Toplevel.itk =================================================================== RCS file: /cvs/src/src/itcl/itk/library/Toplevel.itk,v retrieving revision 1.2 diff -u -r1.2 Toplevel.itk --- itcl/itk/library/Toplevel.itk 22 Feb 2002 02:15:47 -0000 1.2 +++ itcl/itk/library/Toplevel.itk 31 May 2002 23:35:38 -0000 @@ -65,9 +65,19 @@ } itk_component delete hull - # Any remaining components must be outside the hull - foreach component [component] { - destroy [component $component] + # Any remaining components must be outside the hull. + # Loop twice to avoid an error that can happen if + # a component got destroyed as a result of another + # component getting destroyed. + + set components [component] + foreach component $components { + set path($component) [component $component] + } + foreach component $components { + if {[winfo exists $path($component)]} { + destroy $path($component) + } } } Index: itcl/itk/library/Widget.itk =================================================================== RCS file: /cvs/src/src/itcl/itk/library/Widget.itk,v retrieving revision 1.2 diff -u -r1.2 Widget.itk --- itcl/itk/library/Widget.itk 22 Feb 2002 02:15:47 -0000 1.2 +++ itcl/itk/library/Widget.itk 31 May 2002 23:35:38 -0000 @@ -66,9 +66,19 @@ } itk_component delete hull - # Any remaining components must be outside the hull - foreach component [component] { - destroy [component $component] + # Any remaining components must be outside the hull. + # Loop twice to avoid an error that can happen if + # a component got destroyed as a result of another + # component getting destroyed. + + set components [component] + foreach component $components { + set path($component) [component $component] + } + foreach component $components { + if {[winfo exists $path($component)]} { + destroy $path($component) + } } } Index: itcl/itk/tests/toplevel.test =================================================================== RCS file: /cvs/src/src/itcl/itk/tests/toplevel.test,v retrieving revision 1.2 diff -u -r1.2 toplevel.test --- itcl/itk/tests/toplevel.test 22 Feb 2002 02:15:48 -0000 1.2 +++ itcl/itk/tests/toplevel.test 31 May 2002 23:35:39 -0000 @@ -89,15 +89,20 @@ test toplevel-1.8 {when a mega-widget object is deleted, its window and any components are destroyed (even if in another window) } { + catch {destroy .t1} + catch {destroy .t2} + catch {rename .t2 {}} + catch {itcl::delete class ButtonTop} + itcl::class ButtonTop { inherit itk::Toplevel constructor {args} { eval itk_initialize $args - itk_component add button { + itk_component add button { button $itk_option(-container).b -text Button - } {} + } {} pack $itk_component(button) } @@ -106,17 +111,90 @@ toplevel .t1 ButtonTop .t2 -container .t1 - set button [.t2 component button] - itcl::delete object .t2 - set result [list $button [winfo exists $button]] + itcl::delete class ButtonTop + destroy .t1 + set result +} {.t1.b 0} + +test toplevel-1.9 {when a window that contains a megawidget component + is destroyed, the component is removed from the megawidget} { + catch {destroy .t1} + catch {destroy .t2} + catch {rename .t2 {}} + catch {itcl::delete class ButtonTop} + + itcl::class ButtonTop { + inherit itk::Toplevel + constructor {args} { + eval itk_initialize $args + + itk_component add button { + button $itk_option(-container).b -text Button + } {} + pack $itk_component(button) + } + + itk_option define -container container Container {} + } + + toplevel .t1 + ButtonTop .t2 -container .t1 + set result [list [.t2 component]] + destroy .t1 + lappend result [list [.t2 component]] + itcl::delete object .t2 itcl::delete class ButtonTop + set result +} {{button hull} hull} +test toplevel-1.10 {when destroying a component that is inside another + window protect against that case where one component destroy + actually destroys other contained components} { + catch {destroy .t1} + catch {destroy .t2} + catch {rename .t2 {}} + catch {itcl::delete class ButtonTop} + + itcl::class ButtonTop { + inherit itk::Toplevel + + constructor {args} { + eval itk_initialize $args + + # Note, the component names matter here since + # [.t2 component] returns names in hash order. + # We need to delete cframe first since it + # is the parent of cbutton. + + itk_component add cframe { + button $itk_option(-container).cframe + } {} + pack $itk_component(cframe) + + itk_component add cbutton { + button $itk_component(cframe).b -text Button + } {} + pack $itk_component(cbutton) + } + + itk_option define -container container Container {} + } + + toplevel .t1 + ButtonTop .t2 -container .t1 + set result [list [.t2 component]] + # destructor should destroy cframe but not cbutton + itcl::delete object .t2 + lappend result [winfo exists .t1.cframe] + destroy .t1 + itcl::delete class ButtonTop set result -} {.t1.b 0} +} {{hull cframe cbutton} 0} + # ---------------------------------------------------------------------- # Clean up Index: itcl/itk/tests/widget.test =================================================================== RCS file: /cvs/src/src/itcl/itk/tests/widget.test,v retrieving revision 1.2 diff -u -r1.2 widget.test --- itcl/itk/tests/widget.test 22 Feb 2002 02:15:49 -0000 1.2 +++ itcl/itk/tests/widget.test 31 May 2002 23:35:39 -0000 @@ -267,15 +267,19 @@ test widget-1.27 {when a mega-widget object is deleted, its window and any components are destroyed (even if in another window) } { + catch {destroy .t1} + catch {rename .t1.bw {}} + catch {itcl::delete class ButtonWidget} + itcl::class ButtonWidget { inherit itk::Widget constructor {args} { eval itk_initialize $args - itk_component add button { + itk_component add button { button $itk_option(-container).b -text Button - } {} + } {} pack $itk_component(button) } @@ -290,15 +294,97 @@ pack .t1.bw set button [.t1.bw component button] - itcl::delete object .t1.bw - set result [list $button [winfo exists $button]] + destroy .t1 + itcl::delete class ButtonWidget + set result +} {.t1.f.b 0} + +test widget-1.28 {when a window that contains a megawidget component + is destroyed, the component is removed from the megawidget} { + catch {destroy .t1} + catch {rename .t1.bw {}} + catch {itcl::delete class ButtonWidget} + + itcl::class ButtonWidget { + inherit itk::Widget + + constructor {args} { + eval itk_initialize $args + itk_component add button { + button $itk_option(-container).b -text Button + } {} + pack $itk_component(button) + } + + itk_option define -container container Container {} + } + + toplevel .t1 + frame .t1.f + ButtonWidget .t1.bw -container .t1.f + + pack .t1.f + pack .t1.bw + set result [list [.t1.bw component]] + destroy .t1.f + lappend result [list [.t1.bw component]] + + itcl::delete object .t1.bw + destroy .t1 itcl::delete class ButtonWidget + set result +} {{button hull} hull} + +test widget-1.29 {when destroying a component that is inside another + window protect against that case where one component destroy + actually destroys other contained components} { + catch {destroy .t1} + catch {rename .t1.bw {}} + catch {itcl::delete class ButtonWidget} + + itcl::class ButtonWidget { + inherit itk::Widget + constructor {args} { + eval itk_initialize $args + + # Note, the component names matter here since + # [.t2 component] returns names in hash order. + # We need to delete cframe first since it + # is the parent of cbutton. + + itk_component add cframe { + button $itk_option(-container).cframe + } {} + pack $itk_component(cframe) + + itk_component add cbutton { + button $itk_component(cframe).b -text Button + } {} + pack $itk_component(cbutton) + } + + itk_option define -container container Container {} + } + + toplevel .t1 + frame .t1.f + ButtonWidget .t1.bw -container .t1.f + + pack .t1.f + pack .t1.bw + set result [list [.t1.bw component]] + # destructor should destroy cframe but not cbutton + itcl::delete object .t1.bw + lappend result [winfo exists .t1.f.cframe] + + destroy .t1 + itcl::delete class ButtonWidget set result -} {.t1.f.b 0} +} {{hull cframe cbutton} 0} # ----------------------------------------------------------------------