From 1d9aa6aa61da3dd5f1a1a488018ceb8b5a88f2af Mon Sep 17 00:00:00 2001 From: j-p Date: Mon, 28 Aug 2017 05:12:19 +0200 Subject: [PATCH] full IDisposable implementation (#34) * full implementation of IDisposable, not stable, lock and still leaking * override dispose in popper to handle _content not yet in graphic tree (when never shown), test focusedWidget when disposing * prevent canceling thread multiple times causing a deadlock * test parent type when trying to close window --- Tests/InterfaceControler.cs | 2 +- Tests/Tests.csproj | 2 +- src/CrowThread.cs | 8 ++-- src/GraphicObjects/GraphicObject.cs | 57 ++++++++++++++++++++------ src/GraphicObjects/Group.cs | 16 ++++---- src/GraphicObjects/Popper.cs | 9 ++++ src/GraphicObjects/PrivateContainer.cs | 11 ++--- src/GraphicObjects/TemplatedGroup.cs | 16 +++++--- src/GraphicObjects/Window.cs | 7 +++- src/Interface.cs | 8 ---- 10 files changed, 84 insertions(+), 52 deletions(-) diff --git a/Tests/InterfaceControler.cs b/Tests/InterfaceControler.cs index 4d988c10..4da65d1e 100644 --- a/Tests/InterfaceControler.cs +++ b/Tests/InterfaceControler.cs @@ -162,7 +162,7 @@ namespace Crow while (true) { CrowInterface.Update (); - //Thread.Sleep (1); + Thread.Sleep (1); } } diff --git a/Tests/Tests.csproj b/Tests/Tests.csproj index 2adcdb16..21cb740d 100644 --- a/Tests/Tests.csproj +++ b/Tests/Tests.csproj @@ -8,7 +8,7 @@ Exe Tests Tests - Tests.Showcase + Tests.BasicTests v4.5 AnyCPU 0.5 diff --git a/src/CrowThread.cs b/src/CrowThread.cs index fbcdc956..823fe1a6 100644 --- a/src/CrowThread.cs +++ b/src/CrowThread.cs @@ -33,7 +33,7 @@ namespace Crow /// Thread monitored by current interface with Finished event when state==Stopped /// public class CrowThread { - public bool cancel = false; + public bool cancelRequested = false; Thread thread; public event EventHandler Finished; public GraphicObject Host; @@ -53,11 +53,9 @@ namespace Crow } public void Start() { thread.Start();} public void Cancel(){ - if (thread.IsAlive){ - cancel = true; - //cancelLoading = true; + if (thread.IsAlive & !cancelRequested){ + cancelRequested = true; thread.Join (); - //cancelLoading = false; } lock (Host.currentInterface.CrowThreads) Host.currentInterface.CrowThreads.Remove (this); diff --git a/src/GraphicObjects/GraphicObject.cs b/src/GraphicObjects/GraphicObject.cs index 8fd9a6e9..93e812de 100644 --- a/src/GraphicObjects/GraphicObject.cs +++ b/src/GraphicObjects/GraphicObject.cs @@ -41,22 +41,51 @@ namespace Crow public class GraphicObject : ILayoutable, IValueChange, IDisposable { #region IDisposable implementation + protected bool disposed = false; + + public void Dispose(){ + Dispose(true); + GC.SuppressFinalize(this); + } + ~GraphicObject(){ + Dispose(false); + } + protected virtual void Dispose(bool disposing){ + if (disposed){ + #if DEBUG_DISPOSE + Debug.WriteLine ("Trying to dispose already disposed obj: {0}", this.ToString()); + #endif + return; + } - public virtual void Dispose () - { - #if DEBUG_DISPOSE - Debug.WriteLine ("{0} Disposed", this.ToString()); - #endif - - parent = null; - if (IsQueueForRedraw) - Debugger.Break (); - if (!localDataSourceIsNull) - DataSource = null; + if (disposing) { + #if DEBUG_DISPOSE + Debug.WriteLine ("Disposing: {0}", this.ToString()); + if (IsQueueForRedraw) + throw new Exception("Trying to dispose an object queued for Redraw: " + this.ToString()); + #endif + if (currentInterface.HoverWidget != null) { + if (currentInterface.HoverWidget.IsOrIsInside(this)) + currentInterface.HoverWidget = null; + } + if (currentInterface.ActiveWidget != null) { + if (currentInterface.ActiveWidget.IsOrIsInside (this)) + currentInterface.ActiveWidget = null; + } + if (currentInterface.FocusedWidget != null) { + if (currentInterface.FocusedWidget.IsOrIsInside (this)) + currentInterface.FocusedWidget = null; + } + if (!localDataSourceIsNull) + DataSource = null; + parent = null; + } else + Debug.WriteLine ("!!! Finalized by GC: {0}", this.ToString ()); Clipping?.Dispose (); bmp?.Dispose (); - } + disposed = true; + } #endregion internal static ulong currentUid = 0; @@ -826,7 +855,9 @@ namespace Crow /// /// return true if this is contained inside go /// - public bool IsInside(GraphicObject go){ + public bool IsOrIsInside(GraphicObject go){ + if (this == go) + return true; ILayoutable p = this.Parent; while (p != null) { if (p == go) diff --git a/src/GraphicObjects/Group.cs b/src/GraphicObjects/Group.cs index f3026290..e7d7d4d4 100644 --- a/src/GraphicObjects/Group.cs +++ b/src/GraphicObjects/Group.cs @@ -73,13 +73,9 @@ namespace Crow public virtual void RemoveChild(GraphicObject child) { child.LayoutChanged -= OnChildLayoutChanges; - child.Parent = null; //check if HoverWidget is removed from Tree - if (currentInterface.HoverWidget != null) { - if (currentInterface.HoverWidget.IsInside(this)) - currentInterface.HoverWidget = null; - } + lock (children) Children.Remove(child); @@ -366,11 +362,13 @@ namespace Crow } #endregion - public override void Dispose () + protected override void Dispose (bool disposing) { - foreach (GraphicObject c in children) - c.Dispose (); - base.Dispose (); + if (disposing) { + foreach (GraphicObject c in children) + c.Dispose (); + } + base.Dispose (disposing); } } } diff --git a/src/GraphicObjects/Popper.cs b/src/GraphicObjects/Popper.cs index 3ba6d711..d6e36346 100644 --- a/src/GraphicObjects/Popper.cs +++ b/src/GraphicObjects/Popper.cs @@ -240,5 +240,14 @@ namespace Crow } Unpoped.Raise (this, e); } + + protected override void Dispose (bool disposing) + { + if (_content != null && disposing) { + if (_content.Parent == null) + _content.Dispose (); + } + base.Dispose (disposing); + } } } diff --git a/src/GraphicObjects/PrivateContainer.cs b/src/GraphicObjects/PrivateContainer.cs index 89b30753..077f732f 100644 --- a/src/GraphicObjects/PrivateContainer.cs +++ b/src/GraphicObjects/PrivateContainer.cs @@ -54,14 +54,9 @@ namespace Crow if (child != null) { //check if HoverWidget is removed from Tree - if (currentInterface.HoverWidget != null) { - if (currentInterface.HoverWidget.IsInside(this)) - currentInterface.HoverWidget = null; - } contentSize = new Size (0, 0); child.LayoutChanged -= OnChildLayoutChanges; child.Dispose (); - child.Parent = null; this.RegisterForGraphicUpdate (); } @@ -213,11 +208,11 @@ namespace Crow } #endregion - public override void Dispose () + protected override void Dispose (bool disposing) { - if (child != null) + if (disposing && child != null) child.Dispose (); - base.Dispose (); + base.Dispose (disposing); } } } diff --git a/src/GraphicObjects/TemplatedGroup.cs b/src/GraphicObjects/TemplatedGroup.cs index e4e29404..1cbc4935 100644 --- a/src/GraphicObjects/TemplatedGroup.cs +++ b/src/GraphicObjects/TemplatedGroup.cs @@ -273,8 +273,10 @@ namespace Crow ItemTemplates ["default"] = Interface.GetItemTemplate (ItemTemplate); for (int i = 1; i <= (data.Count / itemPerPage) + 1; i++) { - if ((bool)loadingThread?.cancel) + if ((bool)loadingThread?.cancelRequested) { + this.Dispose (); return; + } loadPage (i); Thread.Sleep (1); } @@ -315,8 +317,10 @@ namespace Crow for (int i = (pageNum - 1) * itemPerPage; i < pageNum * itemPerPage; i++) { if (i >= data.Count) break; - if ((bool)loadingThread?.cancel) + if ((bool)loadingThread?.cancelRequested) { + page.Dispose (); return; + } loadItem (i, page); } @@ -336,7 +340,7 @@ namespace Crow string getItempKey(Type dataType, object o){ try { return dataType.GetProperty (_dataTest).GetGetMethod ().Invoke (o, null).ToString(); - } catch (Exception ex) { + } catch { return dataType.FullName; } } @@ -426,11 +430,11 @@ namespace Crow SelectedIndex = data.IndexOf((sender as GraphicObject).DataSource); } - public override void Dispose () + protected override void Dispose (bool disposing) { - if (loadingThread != null) + if (disposing && loadingThread != null) loadingThread.Cancel (); - base.Dispose (); + base.Dispose (disposing); } } } diff --git a/src/GraphicObjects/Window.cs b/src/GraphicObjects/Window.cs index e0c97ea9..176e8d90 100644 --- a/src/GraphicObjects/Window.cs +++ b/src/GraphicObjects/Window.cs @@ -391,7 +391,12 @@ namespace Crow protected void close(){ Closing.Raise (this, null); - currentInterface.DeleteWidget (this); + if (Parent is Interface) + (Parent as Interface).DeleteWidget (this); + else if (Parent is Group) + (Parent as Group).RemoveChild (this); + else if (Parent is PrivateContainer) + (Parent as Container).Child = null; } } } diff --git a/src/Interface.cs b/src/Interface.cs index baa46667..24bd3ca2 100644 --- a/src/Interface.cs +++ b/src/Interface.cs @@ -592,10 +592,6 @@ namespace Crow /// Set visible state of widget to false and remove if from the graphic tree public void DeleteWidget(GraphicObject g) { - if (_hoverWidget != null) { - if (_hoverWidget.IsInside(g)) - HoverWidget = null; - } lock (UpdateMutex) { RegisterClip (g.ScreenCoordinates (g.LastPaintedSlot)); GraphicTree.Remove (g); @@ -611,10 +607,6 @@ namespace Crow //TODO:parent is not reset to null because object will be added //to ObjectToRedraw list, and without parent, it fails GraphicObject g = GraphicTree [0]; - if (_hoverWidget != null) { - if (_hoverWidget.IsInside(g)) - HoverWidget = null; - } RegisterClip (g.ScreenCoordinates (g.LastPaintedSlot)); GraphicTree.RemoveAt (0); g.Dispose (); -- 2.47.3