* Re: Request for patch review.
2002-09-13 7:43 ` Keith Seitz
@ 2002-09-13 15:42 ` Mo DeJong
0 siblings, 0 replies; 3+ messages in thread
From: Mo DeJong @ 2002-09-13 15:42 UTC (permalink / raw)
To: insight
On Fri, 13 Sep 2002 07:46:19 -0700 (PDT)
Keith Seitz <keiths@redhat.com> 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 <supermo@bayarea.net>
* 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}
# ----------------------------------------------------------------------
^ permalink raw reply [flat|nested] 3+ messages in thread