public inbox for insight@sourceware.org
 help / color / mirror / Atom feed
From: Mo DeJong <supermo@bayarea.net>
To: insight@sources.redhat.com
Subject: Re: Request for patch review.
Date: Fri, 13 Sep 2002 15:42:00 -0000	[thread overview]
Message-ID: <20020913154256.063fc8a3.supermo@bayarea.net> (raw)
In-Reply-To: <Pine.LNX.4.44.0209130745510.1407-100000@valrhona.uglyboxes.com>

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}
 
 
 # ----------------------------------------------------------------------

      reply	other threads:[~2002-09-13 22:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-09-12 15:21 Mo DeJong
2002-09-13  7:43 ` Keith Seitz
2002-09-13 15:42   ` Mo DeJong [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20020913154256.063fc8a3.supermo@bayarea.net \
    --to=supermo@bayarea.net \
    --cc=insight@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).