Skip to content

Commit 0a1c634

Browse files
ArtfunkelArtfunkel
committed
ElementArray is now thread safe
Fixed ElementArray not actually storing destub result ElementArray.CopyTo now destubs ImportElement no longer tries to claim an unowned Element if it would cause an ID collision Changed ImportElement arguments to provide more flexibility Bumped version number
1 parent 9b282b3 commit 0a1c634

5 files changed

Lines changed: 129 additions & 64 deletions

File tree

‎Arrays.cs‎

Lines changed: 71 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@
55
using System.Linq;
66
using System.Text;
77
using System.Drawing;
8+
using System.Threading;
89

910
namespace Datamodel
1011
{
1112
public abstract class Array<T> : IList<T>, IList, INotifyCollectionChanged
1213
{
13-
List<T> Inner;
14-
System.Threading.ReaderWriterLockSlim RWLock = new System.Threading.ReaderWriterLockSlim();
14+
protected List<T> Inner;
15+
protected ReaderWriterLockSlim RWLock = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion);
1516
object _SyncRoot = new object();
1617

1718
public virtual Element Owner { get; internal set; }
@@ -157,14 +158,23 @@ public bool Contains(T item)
157158

158159
public void CopyTo(T[] array, int arrayIndex)
159160
{
160-
RWLock.EnterReadLock();
161+
CopyTo_Internal(array, arrayIndex);
162+
}
163+
164+
protected virtual void CopyTo_Internal(Array array, int index)
165+
{
166+
RWLock.EnterUpgradeableReadLock();
161167
try
162168
{
163-
Inner.CopyTo(array, arrayIndex);
169+
foreach (var item in this.Take(Math.Min(array.Length - index, Inner.Count)).ToArray())
170+
{
171+
array.SetValue(item, index);
172+
index++;
173+
}
164174
}
165175
finally
166176
{
167-
RWLock.ExitReadLock();
177+
RWLock.ExitUpgradeableReadLock();
168178
}
169179
}
170180

@@ -216,12 +226,13 @@ public bool Remove(T item)
216226

217227
public IEnumerator<T> GetEnumerator()
218228
{
219-
return Inner.GetEnumerator();
229+
for (int i = 0; i < Inner.Count; i++)
230+
yield return this[i];
220231
}
221232

222233
IEnumerator IEnumerable.GetEnumerator()
223234
{
224-
return Inner.GetEnumerator();
235+
return GetEnumerator();
225236
}
226237

227238
public event NotifyCollectionChangedEventHandler CollectionChanged;
@@ -276,15 +287,7 @@ object IList.this[int index]
276287

277288
void ICollection.CopyTo(Array array, int index)
278289
{
279-
RWLock.EnterReadLock();
280-
try
281-
{
282-
((IList)Inner).CopyTo(array, index);
283-
}
284-
finally
285-
{
286-
RWLock.ExitReadLock();
287-
}
290+
CopyTo_Internal(array, index);
288291
}
289292

290293
bool ICollection.IsSynchronized { get { return true; } }
@@ -314,19 +317,39 @@ public override Element Owner
314317
}
315318
internal set
316319
{
317-
base.Owner = value;
318-
319-
if (OwnerDatamodel != null)
320+
RWLock.EnterUpgradeableReadLock();
321+
try
320322
{
321-
foreach (var elem in this)
323+
base.Owner = value;
324+
325+
if (OwnerDatamodel != null)
322326
{
323-
if (elem == null) continue;
324-
if (elem.Owner == null)
325-
OwnerDatamodel.ImportElement(elem, true, false);
326-
else if (elem.Owner != OwnerDatamodel)
327-
throw new ElementOwnershipException();
327+
for (int i = 0; i < Count; i++)
328+
{
329+
var elem = Inner[i];
330+
331+
if (elem == null) continue;
332+
if (elem.Owner == null)
333+
{
334+
RWLock.EnterWriteLock();
335+
try
336+
{
337+
Inner[i] = OwnerDatamodel.ImportElement(elem, Datamodel.ImportRecursionMode.Stubs, Datamodel.ImportOverwriteMode.Stubs);
338+
}
339+
finally
340+
{
341+
RWLock.ExitWriteLock();
342+
}
343+
}
344+
else if (elem.Owner != OwnerDatamodel)
345+
throw new ElementOwnershipException();
346+
}
328347
}
329348
}
349+
finally
350+
{
351+
RWLock.ExitUpgradeableReadLock();
352+
}
330353
}
331354
}
332355

@@ -337,7 +360,7 @@ protected override void Insert_Internal(int index, Element item)
337360
if (item != null && OwnerDatamodel != null)
338361
{
339362
if (item.Owner == null)
340-
OwnerDatamodel.ImportElement(item, true, false);
363+
OwnerDatamodel.ImportElement(item, Datamodel.ImportRecursionMode.Recursive, Datamodel.ImportOverwriteMode.Stubs);
341364
else if (item.Owner != OwnerDatamodel)
342365
throw new ElementOwnershipException();
343366
}
@@ -347,10 +370,28 @@ public override Element this[int index]
347370
{
348371
get
349372
{
350-
var elem = base[index];
351-
if (elem != null && elem.Stub && elem.Owner != null)
352-
elem = base[index] = elem.Owner.OnStubRequest(elem.ID);
353-
return elem;
373+
RWLock.EnterUpgradeableReadLock();
374+
try
375+
{
376+
var elem = Inner[index];
377+
if (elem != null && elem.Stub && elem.Owner != null)
378+
{
379+
RWLock.EnterWriteLock();
380+
try
381+
{
382+
elem = Inner[index] = elem.Owner.OnStubRequest(elem.ID);
383+
}
384+
finally
385+
{
386+
RWLock.ExitWriteLock();
387+
}
388+
}
389+
return elem;
390+
}
391+
finally
392+
{
393+
RWLock.ExitUpgradeableReadLock();
394+
}
354395
}
355396
set
356397
{

‎Datamodel.cs‎

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ internal Element OnStubRequest(Guid id)
255255
if (result != null && result.ID != id)
256256
throw new InvalidOperationException("Datamodel.StubRequest returned an Element with a an ID different from the one requested.");
257257
if (result.Owner != this)
258-
result = ImportElement(result, false, true);
258+
result = ImportElement(result, ImportRecursionMode.Stubs, ImportOverwriteMode.Stubs);
259259
}
260260

261261
return result ?? AllElements[id];
@@ -572,7 +572,7 @@ public Element Root
572572
if (value != null)
573573
{
574574
if (value.Owner == null)
575-
value = ImportElement(value,true,true);
575+
value = ImportElement(value, ImportRecursionMode.Recursive, ImportOverwriteMode.All);
576576
else if (value.Owner != this)
577577
throw new ElementOwnershipException();
578578
}
@@ -592,53 +592,72 @@ public Element Root
592592
/// <summary>
593593
/// Copies another Datamodel's <see cref="Element"/> into this one.
594594
/// </summary>
595-
/// <remarks>An imported Element will automatically replace any stub Elements which share its ID.</remarks>
595+
/// <remarks>The return value will be owned by this Datamodel. It can be:
596+
/// 1. foreign_element. This is the case when foreign_element has no owner and no Element with the same ID exists in this Datamodel.
597+
/// 2. An existing Element owned by this Datamodel with the same ID as foreign_element.
598+
/// 3. A copy of foreign_element. This is the case when foreign_element already had an owner and a corresponding Element was not found in this Datamodel.</remarks>
596599
/// <param name="foreign_element">The Element to import. Must be owned by a different Datamodel.</param>
597-
/// <param name="deep">Whether to import child Elements as well.</param>
598-
/// <param name="overwrite">If true, foreign Elements will replace local stub Elements which share their ID. If false, the same foregin Elements will be skipped.</param>
599-
/// <returns>A copy of the input Element owned by this Datamodel.</returns>
600+
/// <param name="import_mode">How to respond when foreign_element references other foreign Elements.</param>
601+
/// <param name="overwrite_mode">How to respond when the ID of a foreign Element is already in use in this Datamodel.</param>
602+
/// <returns>foreign_element, a local Element, or a new copy of foreign_element. See Remarks for more details.</returns>
600603
/// <exception cref="ArgumentNullException">Thrown if foreign_element is null.</exception>
601604
/// <exception cref="ElementOwnershipException">Thrown if foreign_element is already owned by this Datamodel.</exception>
602605
/// <exception cref="IndexOutOfRangeException">Thrown when the maximum number of Elements allowed in a Datamodel has been reached.</exception>
603606
/// <seealso cref="Element.Stub"/>
604607
/// <seealso cref="Element.ID"/>
605-
public Element ImportElement(Element foreign_element, bool deep, bool overwrite)
608+
public Element ImportElement(Element foreign_element, ImportRecursionMode import_mode, ImportOverwriteMode overwrite_mode)
606609
{
607610
if (foreign_element == null) throw new ArgumentNullException("element");
608611
if (foreign_element.Owner == this) throw new ElementOwnershipException("Element is already a part of this Datamodel.");
609612

610-
ImportMode mode = ImportMode.Normal;
611-
if (deep) mode |= ImportMode.Deep;
612-
if (overwrite) mode |= ImportMode.Overwrite;
613-
614-
return ImportElement_internal(foreign_element, new ImportJob(mode));
613+
return ImportElement_internal(foreign_element, new ImportJob(import_mode, overwrite_mode));
615614
}
616615

617-
[Flags]
618-
enum ImportMode
616+
public enum ImportRecursionMode
619617
{
620-
Normal,
618+
/// <summary>
619+
/// Import the given Element only. Any Element reference will become null.
620+
/// </summary>
621+
Nulls,
622+
/// <summary>
623+
/// Import the given Element only. Any Element references will be represented with stubs.
624+
/// </summary>
625+
Stubs,
621626
/// <summary>
622627
/// Recursively import all referenced Elements.
623628
/// </summary>
624-
Deep,
629+
Recursive,
630+
}
631+
632+
public enum ImportOverwriteMode
633+
{
625634
/// <summary>
626-
/// Replace local Elements, even if they are not stubs.
635+
/// If a local Element has the same ID as a foreign Element, ignore the foreign Element.
627636
/// </summary>
628-
Overwrite
637+
None,
638+
/// <summary>
639+
/// If a local stub Element has the same ID as a non-stub foreign Element, replace it.
640+
/// </summary>
641+
Stubs,
642+
/// <summary>
643+
/// If a local Element has the same ID as a non-stub foreign Element, replace it.
644+
/// </summary>
645+
All,
629646
}
630647

631648
struct ImportJob
632649
{
633-
public ImportMode Mode;
634-
public Dictionary<Element, Element> ImportMap;
635-
public int Depth;
650+
public ImportRecursionMode ImportMode { get; private set; }
651+
public ImportOverwriteMode OverwriteMode { get; private set; }
652+
public Dictionary<Element, Element> ImportMap { get; private set; }
653+
public int Depth { get; set; }
636654

637-
public ImportJob(ImportMode mode)
655+
public ImportJob(ImportRecursionMode import_mode, ImportOverwriteMode overwrite_mode)
656+
:this()
638657
{
639-
Mode = mode;
658+
ImportMode = import_mode;
659+
OverwriteMode = overwrite_mode;
640660
ImportMap = new Dictionary<Element, Element>();
641-
Depth = 0;
642661
}
643662
}
644663

@@ -660,14 +679,14 @@ object CopyValue(object value, ImportJob job)
660679

661680
if (local_element != null && !local_element.Stub)
662681
best_element = local_element;
663-
else if (!foreign_element.Stub && job.Mode.HasFlag(ImportMode.Deep))
682+
else if (!foreign_element.Stub && job.ImportMode == ImportRecursionMode.Recursive)
664683
{
665684
job.Depth++;
666685
best_element = ImportElement_internal(foreign_element, job);
667686
job.Depth--;
668687
}
669688
else
670-
best_element = local_element ?? new Element(this, foreign_element.ID);
689+
best_element = local_element ?? (job.ImportMode == ImportRecursionMode.Stubs ? new Element(this, foreign_element.ID) : (Element)null);
671690

672691
return best_element;
673692
}
@@ -696,7 +715,7 @@ Element ImportElement_internal(Element foreign_element, ImportJob job)
696715
lock (foreign_element.SyncRoot)
697716
{
698717
// Claim an unowned Element
699-
if (foreign_element.Owner == null)
718+
if (foreign_element.Owner == null && AllElements[foreign_element.ID] == null)
700719
{
701720
foreign_element.Owner = this;
702721
foreach (var attr in foreign_element)
@@ -724,7 +743,7 @@ Element ImportElement_internal(Element foreign_element, ImportJob job)
724743
local_element = AllElements[foreign_element.ID];
725744
if (local_element != null)
726745
{
727-
if (!foreign_element.Stub && (local_element.Stub || job.Mode.HasFlag(ImportMode.Overwrite)))
746+
if (!foreign_element.Stub && job.OverwriteMode == ImportOverwriteMode.All || (job.OverwriteMode == ImportOverwriteMode.Stubs && local_element.Stub))
728747
{
729748
local_element.Name = foreign_element.Name;
730749
local_element.ClassName = foreign_element.ClassName;
@@ -736,15 +755,19 @@ Element ImportElement_internal(Element foreign_element, ImportJob job)
736755
else
737756
{
738757
// Create a new local Element
739-
if (foreign_element.Stub || (!job.Mode.HasFlag(ImportMode.Deep) && job.Depth > 0))
758+
if (foreign_element.Stub || (job.ImportMode == ImportRecursionMode.Stubs && job.Depth > 0))
740759
local_element = new Element(this, foreign_element.ID);
741-
else
760+
else if (job.ImportMode == ImportRecursionMode.Recursive || job.Depth == 0)
742761
local_element = new Element(this, foreign_element.Name, foreign_element.ID, foreign_element.ClassName);
762+
else
763+
local_element = null;
743764
}
744765
job.ImportMap.Add(foreign_element, local_element);
745766

746767
// Copy attributes
747-
if (!local_element.Stub)
768+
if (local_element != null && !local_element.Stub)
769+
{
770+
local_element.Clear();
748771
foreach (var attr in foreign_element)
749772
{
750773
if (attr.Value == null)
@@ -763,6 +786,7 @@ Element ImportElement_internal(Element foreign_element, ImportJob job)
763786
else
764787
local_element[attr.Key] = CopyValue(attr.Value, job);
765788
}
789+
}
766790
return local_element;
767791
}
768792
}

‎Properties/AssemblyInfo.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
[assembly: ComVisible(false)]
1616
[assembly: Guid("2df8c991-dfe3-4d62-9fae-705c73ca54db")]
1717

18-
[assembly: AssemblyVersion("1.1.0.0")]
18+
[assembly: AssemblyVersion("1.2.0.0")]
1919

2020
[assembly: System.CLSCompliant(true)]
2121

‎Tests/Resources/xaml_dm.xaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<Datamodel xmlns="clr-namespace:Datamodel;assembly=Datamodel.NET" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib"
22
Format="View">
33
<Datamodel.Root>
4-
<Element>
4+
<Element Name="Root">
55
<Element x:Key="NonStub" Name="NonStub Element" ID="22DF5745-641A-434D-9722-CAAF2D8CEE28">
66
<s:String x:Key="UTF8">こんいちは</s:String>
77
</Element>

‎Tests/Tests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ public void Import()
241241
Populate(dm, 2);
242242

243243
var dm2 = MakeDatamodel();
244-
dm2.Root = dm2.ImportElement(dm.Root, true, true);
244+
dm2.Root = dm2.ImportElement(dm.Root, DM.ImportRecursionMode.Recursive, DM.ImportOverwriteMode.All);
245245

246246
SaveAndConvert(dm, "keyvalues2", 1);
247247
SaveAndConvert(dm, "binary", 5);

0 commit comments

Comments
 (0)