Skip to content

Commit

Permalink
Fix threadsafety in DataServiceClient EntityTracker's internal dicti…
Browse files Browse the repository at this point in the history
…onary (#1200)

* Change Dictionary<> objects in EntityTracker to use thread-safe ConcurrentDictionary<> objects instead.

* Convert tabs to spaces.

* More tabs to spaces.

* Fix editor removing spaces.

* Fix spaces.

* Mimic IDictionary Add() duplicate exception semantics.

* Fix thread safety
  • Loading branch information
mikepizzo authored and biaol-odata committed Jun 26, 2018
1 parent 135da80 commit 5395430
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 5 deletions.
22 changes: 20 additions & 2 deletions src/Microsoft.OData.Client/DictionaryExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ namespace Microsoft.OData.Client
{
using System;
using System.Collections.Generic;
using System.Collections.Concurrent;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;

/// <summary>
/// Useful extension methods for IDictionary
/// Useful extension methods for IDictionary and ConcurrentDictionary
/// </summary>
internal static class DictionaryExtensions
{
Expand Down Expand Up @@ -54,7 +55,7 @@ internal static void SetRange<TKey, TValue>(this IDictionary<TKey, TValue> dicti
}
}

#if PORTABLELIB
#if PORTABLELIB && WINDOWSPHONE
/// <summary>
/// Try to add a key/value pair to the dictionary and returns true if the key was added, false if it was already present.
/// The difference with Add is that it will not throw <see cref="ArgumentException"/> if the key is already present.
Expand Down Expand Up @@ -85,6 +86,23 @@ internal static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dict, T

return false;
}
#else
/// <summary>
/// Convenience function that wraps ConcurrentDictionary.TryAdd() to allow same signature as IDictionary
/// </summary>
public static void Add<TKey, TValue>(this ConcurrentDictionary<TKey, TValue> self, TKey key, TValue value)
{
if (!self.TryAdd(key, value))
throw new ArgumentException("Argument_AddingDuplicate");
}

/// <summary>
/// Convenience function for ConcurrentDictionary to allow same signature as IDictionary
/// </summary>
public static bool Remove<TKey, TValue>(this ConcurrentDictionary<TKey, TValue> self, TKey key)
{
return ((IDictionary<TKey, TValue>)self).Remove(key);
}
#endif
}
}
29 changes: 26 additions & 3 deletions src/Microsoft.OData.Client/EntityTracker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ namespace Microsoft.OData.Client
#region Namespaces
using System;
using System.Collections.Generic;
using System.Collections.Concurrent;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
Expand All @@ -26,14 +27,28 @@ public class EntityTracker : EntityTrackerBase

#region Resource state management

#if PORTABLELIB && WINDOWSPHONE
/// <summary>Set of tracked resources</summary>
private Dictionary<object, EntityDescriptor> entityDescriptors = new Dictionary<object, EntityDescriptor>(EqualityComparer<object>.Default);

#else
/// <summary>Set of tracked resources</summary>
private ConcurrentDictionary<object, EntityDescriptor> entityDescriptors = new ConcurrentDictionary<object, EntityDescriptor>(EqualityComparer<object>.Default);
#endif
#if PORTABLELIB && WINDOWSPHONE
/// <summary>Set of tracked resources by Identity</summary>
private Dictionary<Uri, EntityDescriptor> identityToDescriptor;
#else
/// <summary>Set of tracked resources by Identity</summary>
private ConcurrentDictionary<Uri, EntityDescriptor> identityToDescriptor;
#endif

#if PORTABLELIB && WINDOWSPHONE
/// <summary>Set of tracked bindings</summary>
private Dictionary<LinkDescriptor, LinkDescriptor> bindings;
#else
/// <summary>Set of tracked bindings</summary>
private ConcurrentDictionary<LinkDescriptor, LinkDescriptor> bindings;
#endif

/// <summary>change order</summary>
private uint nextChange;
Expand Down Expand Up @@ -525,16 +540,24 @@ private void EnsureIdentityToResource()
{
if (null == this.identityToDescriptor)
{
#if PORTABLELIB && WINDOWSPHONE
System.Threading.Interlocked.CompareExchange(ref this.identityToDescriptor, new Dictionary<Uri, EntityDescriptor>(EqualityComparer<Uri>.Default), null);
}
}
#else
System.Threading.Interlocked.CompareExchange(ref this.identityToDescriptor, new ConcurrentDictionary<Uri, EntityDescriptor>(EqualityComparer<Uri>.Default), null);
#endif
}
}

/// <summary>create this.bindings when necessary</summary>
private void EnsureLinkBindings()
{
if (null == this.bindings)
{
#if PORTABLELIB && WINDOWSPHONE
System.Threading.Interlocked.CompareExchange(ref this.bindings, new Dictionary<LinkDescriptor, LinkDescriptor>(LinkDescriptor.EquivalenceComparer), null);
#else
System.Threading.Interlocked.CompareExchange(ref this.bindings, new ConcurrentDictionary<LinkDescriptor, LinkDescriptor>(LinkDescriptor.EquivalenceComparer), null);
#endif
}
}

Expand Down

0 comments on commit 5395430

Please sign in to comment.