From 05e7cd5133826afd42d215e08c4874ee48ed49be Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jean-Philippe=20Bruy=C3=A8re?= Date: Wed, 21 Feb 2018 16:11:30 +0100 Subject: [PATCH] implement ReaderWritelLockSlim to resolve threading issues --- src/GraphicObjects/Docker.cs | 14 ++-- src/GraphicObjects/GraphicObject.cs | 43 +++++----- src/GraphicObjects/Group.cs | 125 +++++++++++++++++----------- src/GraphicObjects/TabView.cs | 21 +++-- src/GraphicObjects/Wrapper.cs | 82 ++++++++++-------- src/Instantiator.cs | 2 +- src/Interface.cs | 20 +++-- src/LayoutingQueueItem.cs | 12 ++- 8 files changed, 193 insertions(+), 126 deletions(-) diff --git a/src/GraphicObjects/Docker.cs b/src/GraphicObjects/Docker.cs index 3b66eb7e..c52c094f 100644 --- a/src/GraphicObjects/Docker.cs +++ b/src/GraphicObjects/Docker.cs @@ -146,14 +146,16 @@ namespace Crow gr.Clip (); } - lock (Children) { - foreach (GraphicObject g in Children) { - if (CurrentInterface.DragAndDropOperation?.DragSource == g) - continue; - g.Paint (ref gr); - } + childrenRWLock.EnterReadLock (); + + foreach (GraphicObject g in Children) { + if (CurrentInterface.DragAndDropOperation?.DragSource == g) + continue; + g.Paint (ref gr); } + childrenRWLock.ExitReadLock (); + if (showDockingTarget) { Rectangle r; if (mainStack == null) diff --git a/src/GraphicObjects/GraphicObject.cs b/src/GraphicObjects/GraphicObject.cs index d49cbe8f..f671239d 100644 --- a/src/GraphicObjects/GraphicObject.cs +++ b/src/GraphicObjects/GraphicObject.cs @@ -34,6 +34,7 @@ using System.Runtime.CompilerServices; using Cairo; using System.Diagnostics; using Crow.IML; +using System.Threading; namespace Crow { @@ -42,6 +43,8 @@ namespace Crow /// public class GraphicObject : ILayoutable, IValueChange, IDisposable { + internal ReaderWriterLockSlim parentRWLock = new ReaderWriterLockSlim(); + #region IDisposable implementation protected bool disposed = false; @@ -66,6 +69,7 @@ namespace Crow 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; @@ -80,13 +84,15 @@ namespace Crow } if (!localDataSourceIsNull) DataSource = null; + + parentRWLock.EnterWriteLock(); parent = null; + parentRWLock.ExitWriteLock(); } else Debug.WriteLine ("!!! Finalized by GC: {0}", this.ToString ()); Clipping?.Dispose (); bmp?.Dispose (); disposed = true; - } #endregion @@ -217,7 +223,7 @@ namespace Crow //TODO: we should ensure the whole parsed widget tree is the last painted public Rectangle LastPaintedSlot; /// Prevent requeuing multiple times the same widget - public bool IsQueueForRedraw = false; + public bool IsQueueForClipping = false; /// drawing Cache, if null, a redraw is done, cached or not public Surface bmp; public bool IsDirty = true; @@ -243,11 +249,12 @@ namespace Crow if (parent == value) return; DataSourceChangeEventArgs e = new DataSourceChangeEventArgs (parent, value); - lock (CurrentInterface.LayoutMutex) { - parent = value; - } - onParentChanged (this, e); - + + parentRWLock.EnterWriteLock(); + parent = value; + parentRWLock.ExitWriteLock(); + + onParentChanged (this, e); } } [XmlIgnore]public ILayoutable LogicalParent { @@ -774,7 +781,7 @@ namespace Crow /// Seek first logical tree upward if logicalParent is set, or seek graphic tree for /// a not null dataSource that will be active for all descendants having dataSource=null /// - [XmlAttributeAttribute]//[DefaultValue(null)] + [XmlAttributeAttribute] public virtual object DataSource { set { if (DataSource == value) @@ -784,12 +791,11 @@ namespace Crow dataSource = value; dse.NewDataSource = DataSource; - //prevent setting null causing stack overflow in specific case if (dse.NewDataSource == dse.OldDataSource) return; - lock (CurrentInterface.LayoutMutex) { - OnDataSourceChanged (this, dse); - } + + OnDataSourceChanged (this, dse); + NotifyValueChanged ("DataSource", DataSource); } @@ -854,9 +860,9 @@ namespace Crow /// Loads the default values from XML attributes default public void loadDefaultValues() { - #if DEBUG_LOAD - Debug.WriteLine ("LoadDefValues for " + this.ToString ()); - #endif +// #if DEBUG_LOAD +// Debug.WriteLine ("LoadDefValues for " + this.ToString ()); +// #endif Type thisType = this.GetType (); @@ -1138,13 +1144,12 @@ namespace Crow #if DEBUG_UPDATE Debug.WriteLine (string.Format("ClippingRegistration -> {0}", this.ToString ())); #endif - lock(CurrentInterface.LayoutMutex){ - IsQueueForRedraw = false; - if (Parent == null) - return; + parentRWLock.EnterReadLock (); + if (parent != null) { Parent.RegisterClip (LastPaintedSlot); Parent.RegisterClip (Slot); } + parentRWLock.ExitReadLock (); } /// /// Add clip rectangle to this.clipping and propagate up to root diff --git a/src/GraphicObjects/Group.cs b/src/GraphicObjects/Group.cs index 1b5847d4..e7604842 100644 --- a/src/GraphicObjects/Group.cs +++ b/src/GraphicObjects/Group.cs @@ -31,12 +31,15 @@ using System.Xml.Serialization; using Cairo; using System.Diagnostics; using System.Reflection; +using System.Threading; namespace Crow { public class Group : GraphicObject { + protected ReaderWriterLockSlim childrenRWLock = new ReaderWriterLockSlim(); + #region CTOR public Group () : base() {} public Group(Interface iface) : base(iface){} @@ -62,10 +65,13 @@ namespace Crow set { _multiSelect = value; } } public virtual void AddChild(GraphicObject g){ - lock (Children) { - g.Parent = this; - Children.Add (g); - } + childrenRWLock.EnterWriteLock(); + + g.Parent = this; + Children.Add (g); + + childrenRWLock.ExitWriteLock(); + g.RegisteredLayoutings = LayoutingType.None; g.LayoutChanged += OnChildLayoutChanges; g.RegisterForLayouting (LayoutingType.Sizing | LayoutingType.ArrangeChildren); @@ -79,8 +85,11 @@ namespace Crow CurrentInterface.HoverWidget = null; } - lock (Children) - Children.Remove(child); + childrenRWLock.EnterWriteLock (); + + Children.Remove(child); + + childrenRWLock.ExitWriteLock (); if (child == largestChild && Width == Measure.Fit) searchLargestChild (); @@ -96,10 +105,13 @@ namespace Crow child.Dispose (); } public virtual void InsertChild (int idx, GraphicObject g) { - lock (Children) { - g.Parent = this; - Children.Insert (idx, g); - } + childrenRWLock.EnterWriteLock (); + + g.Parent = this; + Children.Insert (idx, g); + + childrenRWLock.ExitWriteLock (); + g.RegisteredLayoutings = LayoutingType.None; g.LayoutChanged += OnChildLayoutChanges; g.RegisterForLayouting (LayoutingType.Sizing | LayoutingType.ArrangeChildren); @@ -109,15 +121,17 @@ namespace Crow } public virtual void ClearChildren() { - lock (Children) { - while (Children.Count > 0) { - GraphicObject g = Children [Children.Count - 1]; - g.LayoutChanged -= OnChildLayoutChanges; - Children.RemoveAt (Children.Count - 1); - g.Dispose (); - } + childrenRWLock.EnterWriteLock (); + + while (Children.Count > 0) { + GraphicObject g = Children [Children.Count - 1]; + g.LayoutChanged -= OnChildLayoutChanges; + Children.RemoveAt (Children.Count - 1); + g.Dispose (); } + childrenRWLock.ExitWriteLock (); + resetChildrenMaxSize (); this.RegisterForLayouting (LayoutingType.Sizing); @@ -128,20 +142,24 @@ namespace Crow { if (Children.Contains(w)) { - lock (Children) { - Children.Remove (w); - Children.Add (w); - } + childrenRWLock.EnterWriteLock (); + + Children.Remove (w); + Children.Add (w); + + childrenRWLock.ExitWriteLock (); } } public void putWidgetOnBottom(GraphicObject w) { if (Children.Contains(w)) { - lock (Children) { - Children.Remove (w); - Children.Insert (0, w); - } + childrenRWLock.EnterWriteLock (); + + Children.Remove (w); + Children.Insert (0, w); + + childrenRWLock.ExitWriteLock (); } } @@ -149,24 +167,31 @@ namespace Crow public override void OnDataSourceChanged (object sender, DataSourceChangeEventArgs e) { base.OnDataSourceChanged (this, e); - lock (Children) { - foreach (GraphicObject g in children) - if (g.localDataSourceIsNull & g.localLogicalParentIsNull) - g.OnDataSourceChanged (g, e); - } + + childrenRWLock.EnterReadLock (); + + foreach (GraphicObject g in children) + if (g.localDataSourceIsNull & g.localLogicalParentIsNull) + g.OnDataSourceChanged (g, e); + + childrenRWLock.ExitReadLock (); } public override GraphicObject FindByName (string nameToFind) { if (Name == nameToFind) return this; GraphicObject tmp = null; - lock (Children) { - foreach (GraphicObject w in Children) { - tmp = w.FindByName (nameToFind); - if (tmp != null) - break; - } + + childrenRWLock.EnterReadLock (); + + foreach (GraphicObject w in Children) { + tmp = w.FindByName (nameToFind); + if (tmp != null) + break; } + + childrenRWLock.ExitReadLock (); + return tmp; } public override bool Contains (GraphicObject goToFind) @@ -207,10 +232,11 @@ namespace Crow { base.OnLayoutChanges (layoutType); + childrenRWLock.EnterReadLock (); //position smaller objects in group when group size is fit switch (layoutType) { case LayoutingType.Width: - foreach (GraphicObject c in Children){ + foreach (GraphicObject c in Children) { if (c.Width.Units == Unit.Percent) c.RegisterForLayouting (LayoutingType.Width); else @@ -226,6 +252,7 @@ namespace Crow } break; } + childrenRWLock.ExitReadLock (); } protected override void onDraw (Context gr) { @@ -239,11 +266,13 @@ namespace Crow gr.Clip (); } - lock (Children) { + childrenRWLock.EnterReadLock (); + foreach (GraphicObject g in Children) { g.Paint (ref gr); } - } + + childrenRWLock.ExitReadLock (); gr.Restore (); } protected override void UpdateCache (Context ctx) @@ -268,16 +297,18 @@ namespace Crow gr.Clip (); } - lock (Children) { - foreach (GraphicObject c in Children) { - if (!c.Visible) - continue; - if (Clipping.Contains (c.Slot + ClientRectangle.Position) == RegionOverlap.Out) - continue; - c.Paint (ref gr); - } + childrenRWLock.EnterReadLock (); + + foreach (GraphicObject c in Children) { + if (!c.Visible) + continue; + if (Clipping.Contains (c.Slot + ClientRectangle.Position) == RegionOverlap.Out) + continue; + c.Paint (ref gr); } + childrenRWLock.ExitReadLock (); + #if DEBUG_CLIP_RECTANGLE Clipping.stroke (gr, Color.Amaranth.AdjustAlpha (0.8)); #endif diff --git a/src/GraphicObjects/TabView.cs b/src/GraphicObjects/TabView.cs index c5292da7..abaa6ac0 100644 --- a/src/GraphicObjects/TabView.cs +++ b/src/GraphicObjects/TabView.cs @@ -176,17 +176,20 @@ namespace Crow gr.Clip (); } - lock (Children) { - TabItem[] tabItms = Children.Cast ().OrderBy (t => t.ViewIndex).ToArray (); - for (int i = 0; i < tabItms.Length; i++) { - if (tabItms [i] == Children [SelectedTab]) - continue; - tabItms [i].Paint (ref gr); - } + childrenRWLock.EnterReadLock (); - if (SelectedTab < tabItms.Length && SelectedTab >= 0) - Children [SelectedTab].Paint (ref gr); + TabItem[] tabItms = Children.Cast ().OrderBy (t => t.ViewIndex).ToArray (); + for (int i = 0; i < tabItms.Length; i++) { + if (tabItms [i] == Children [SelectedTab]) + continue; + tabItms [i].Paint (ref gr); } + + if (SelectedTab < tabItms.Length && SelectedTab >= 0) + Children [SelectedTab].Paint (ref gr); + + childrenRWLock.ExitReadLock (); + gr.Restore (); } diff --git a/src/GraphicObjects/Wrapper.cs b/src/GraphicObjects/Wrapper.cs index e56d9332..4fff17ff 100644 --- a/src/GraphicObjects/Wrapper.cs +++ b/src/GraphicObjects/Wrapper.cs @@ -131,26 +131,32 @@ namespace Crow else { int dy = 0; int largestChild = 0; - lock (Children) { - foreach (GraphicObject c in Children) { - if (!c.Visible) - continue; - if (c.Height.Units == Unit.Percent && - c.RegisteredLayoutings.HasFlag (LayoutingType.Height)) - return -1; - if (dy + c.Slot.Height > ClientRectangle.Height) { - dy = 0; - tmp += largestChild + Spacing; - largestChild = c.Slot.Width; - } else if (largestChild < c.Slot.Width) - largestChild = c.Slot.Width; - - dy += c.Slot.Height + Spacing; + + childrenRWLock.EnterReadLock(); + + foreach (GraphicObject c in Children) { + if (!c.Visible) + continue; + if (c.Height.Units == Unit.Percent && + c.RegisteredLayoutings.HasFlag (LayoutingType.Height)) { + childrenRWLock.ExitReadLock(); + return -1; } - if (dy == 0) - tmp -= Spacing; - return tmp + largestChild + 2 * Margin; + if (dy + c.Slot.Height > ClientRectangle.Height) { + dy = 0; + tmp += largestChild + Spacing; + largestChild = c.Slot.Width; + } else if (largestChild < c.Slot.Width) + largestChild = c.Slot.Width; + + dy += c.Slot.Height + Spacing; } + + childrenRWLock.ExitReadLock (); + + if (dy == 0) + tmp -= Spacing; + return tmp + largestChild + 2 * Margin; } } else if (Orientation == Orientation.Horizontal) { Height = Measure.Stretched; @@ -160,26 +166,32 @@ namespace Crow else { int dx = 0; int tallestChild = 0; - lock (Children) { - foreach (GraphicObject c in Children) { - if (!c.Visible) - continue; - if (c.Width.Units == Unit.Percent && - c.RegisteredLayoutings.HasFlag (LayoutingType.Width)) - return -1; - if (dx + c.Slot.Width > ClientRectangle.Width) { - dx = 0; - tmp += tallestChild + Spacing; - tallestChild = c.Slot.Height; - } else if (tallestChild < c.Slot.Height) - tallestChild = c.Slot.Height; - dx += c.Slot.Width + Spacing; + childrenRWLock.EnterReadLock(); + + foreach (GraphicObject c in Children) { + if (!c.Visible) + continue; + if (c.Width.Units == Unit.Percent && + c.RegisteredLayoutings.HasFlag (LayoutingType.Width)) { + childrenRWLock.ExitReadLock(); + return -1; } - if (dx == 0) - tmp -= Spacing; - return tmp + tallestChild + 2 * Margin; + if (dx + c.Slot.Width > ClientRectangle.Width) { + dx = 0; + tmp += tallestChild + Spacing; + tallestChild = c.Slot.Height; + } else if (tallestChild < c.Slot.Height) + tallestChild = c.Slot.Height; + + dx += c.Slot.Width + Spacing; } + + childrenRWLock.ExitReadLock(); + + if (dx == 0) + tmp -= Spacing; + return tmp + tallestChild + 2 * Margin; } } diff --git a/src/Instantiator.cs b/src/Instantiator.cs index b01052ca..14e33bf0 100644 --- a/src/Instantiator.cs +++ b/src/Instantiator.cs @@ -101,7 +101,7 @@ namespace Crow.IML #if DEBUG_LOAD loadingTime.Stop (); Debug.WriteLine ("IML Instantiator creation '{2}' : {0} ticks, {1} ms", - loadingTime.ElapsedTicks, loadingTime.ElapsedMilliseconds, imlPath); + loadingTime.ElapsedTicks, loadingTime.ElapsedMilliseconds, sourcePath); #endif } } diff --git a/src/Interface.cs b/src/Interface.cs index ecc78de5..8c3792d5 100644 --- a/src/Interface.cs +++ b/src/Interface.cs @@ -189,6 +189,8 @@ namespace Crow public object RenderMutex = new object(); /// Global lock of the update cycle public object UpdateMutex = new object(); + /// Global lock of the clipping queue + public object ClippingMutex = new object(); //TODO:share resource instances /// /// Store loaded resources instances shared among controls to reduce memory footprint @@ -199,7 +201,7 @@ namespace Crow /// Store discarded lqi between two updates public Queue DiscardQueue; /// Main drawing queue, holding layouted controls - public Queue DrawingQueue = new Queue(); + public Queue ClippingQueue = new Queue(); public string Clipboard;//TODO:use object instead for complex copy paste /// each IML and fragments (such as inline Templates) are compiled as a Dynamic Method stored here /// on the first instance creation of a IML item. @@ -475,11 +477,11 @@ namespace Crow #if DEBUG_UPDATE Debug.WriteLine (string.Format("\tEnqueueForRepaint -> {0}", g?.ToString ())); #endif - lock (DrawingQueue) { - if (g.IsQueueForRedraw) + lock (ClippingMutex) { + if (g.IsQueueForClipping) return; - DrawingQueue.Enqueue (g); - g.IsQueueForRedraw = true; + ClippingQueue.Enqueue (g); + g.IsQueueForClipping = true; } } /// Main Update loop, executed in this interface thread, protected by the UpdateMutex @@ -588,9 +590,11 @@ namespace Crow clippingMeasure.StartCycle(); #endif GraphicObject g = null; - while (DrawingQueue.Count > 0) { - lock (DrawingQueue) - g = DrawingQueue.Dequeue (); + while (ClippingQueue.Count > 0) { + lock (ClippingMutex) { + g = ClippingQueue.Dequeue (); + g.IsQueueForClipping = false; + } g.ClippingRegistration (); } diff --git a/src/LayoutingQueueItem.cs b/src/LayoutingQueueItem.cs index e87c4146..ba2f95af 100644 --- a/src/LayoutingQueueItem.cs +++ b/src/LayoutingQueueItem.cs @@ -97,12 +97,21 @@ namespace Crow public void ProcessLayouting() { - if (Layoutable.Parent == null) {//TODO:improve this + GraphicObject go = Layoutable as GraphicObject; + if (go == null) { + Debug.WriteLine ("ERROR: processLayouting on something else than a graphic object: " + this.ToString ()); + return; + } + + go.parentRWLock.EnterReadLock (); + + if (go.Parent == null) {//TODO:improve this //cancel layouting for object without parent, maybe some were in queue when //removed from a listbox #if DEBUG_UPDATE || DEBUG_LAYOUTING Debug.WriteLine ("ERROR: processLayouting, no parent for: " + this.ToString ()); #endif + go.parentRWLock.ExitReadLock (); return; } #if DEBUG_LAYOUTING @@ -138,6 +147,7 @@ namespace Crow } LQITime.Stop(); #endif + go.parentRWLock.ExitReadLock (); } public static implicit operator GraphicObject(LayoutingQueueItem queueItem) -- 2.47.3