Skip to content

Commit

Permalink
implement tests to prevent syncronizecontext deadlocks (#18)
Browse files Browse the repository at this point in the history
Co-authored-by: andrew-cat <[email protected]>
  • Loading branch information
endret and andrew-cat authored Jan 7, 2021
1 parent 4e63e5b commit 6b7b24c
Show file tree
Hide file tree
Showing 5 changed files with 243 additions and 18 deletions.
89 changes: 89 additions & 0 deletions src/ConfigCat.Client.Tests/ConfigCatClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,95 @@ public void Dispose_ConfigServiceIsDisposable_ShouldInvokeDispose()
Assert.AreEqual(1, myMock.DisposeCount);
}

[TestMethod]
public void ForceRefresh_ShouldInvokeConfigServiceRefreshConfigAsync()
{
// Arrange

configServiceMock.Setup(m => m.RefreshConfigAsync()).Returns(Task.CompletedTask);

IConfigCatClient instance = new ConfigCatClient(
configServiceMock.Object,
loggerMock.Object,
evaluatorMock.Object,
configDeserializerMock.Object);

// Act

instance.ForceRefresh();

// Assert

configServiceMock.Verify(m => m.RefreshConfigAsync(), Times.Once);
}

[TestMethod]
public async Task ForceRefreshAsync_ShouldInvokeConfigServiceRefreshConfigAsync()
{
// Arrange

configServiceMock.Setup(m => m.RefreshConfigAsync()).Returns(Task.CompletedTask);

IConfigCatClient instance = new ConfigCatClient(
configServiceMock.Object,
loggerMock.Object,
evaluatorMock.Object,
configDeserializerMock.Object);

// Act

await instance.ForceRefreshAsync();

// Assert

configServiceMock.Verify(m => m.RefreshConfigAsync(), Times.Once);
}


[TestMethod]
public void ForceRefresh_ConfigServiceThrowException_ShouldNotReThrowTheExceptionAndLogsError()
{
// Arrange

configServiceMock.Setup(m => m.RefreshConfigAsync()).Throws<Exception>();

IConfigCatClient instance = new ConfigCatClient(
configServiceMock.Object,
loggerMock.Object,
evaluatorMock.Object,
configDeserializerMock.Object);

// Act

instance.ForceRefresh();

// Assert

loggerMock.Verify(m => m.Error(It.IsAny<string>()), Times.Once);
}

[TestMethod]
public async Task ForceRefreshAsync_ConfigServiceThrowException_ShouldNotReThrowTheExceptionAndLogsError()
{
// Arrange

configServiceMock.Setup(m => m.RefreshConfigAsync()).Throws<Exception>();

IConfigCatClient instance = new ConfigCatClient(
configServiceMock.Object,
loggerMock.Object,
evaluatorMock.Object,
configDeserializerMock.Object);

// Act

await instance.ForceRefreshAsync();

// Assert

loggerMock.Verify(m => m.Error(It.IsAny<string>()), Times.Once);
}

internal class FakeConfigService : ConfigServiceBase, IConfigService
{
public byte DisposeCount { get; private set; }
Expand Down
114 changes: 114 additions & 0 deletions src/ConfigCat.Client.Tests/SynchronizationContextDeadlockTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using System;
using System.Threading;
using Moq;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;

namespace ConfigCat.Client.Tests
{
[TestClass]
public class SynchronizationContextDeadlockTests
{
private const string SDKKEY = "PKDVCLf-Hq-h-kCzMp-L7Q/psuH7BGHoUmdONrzzUOY7A";

private readonly Mock<SynchronizationContext> syncContextMock;

private static SynchronizationContext synchronizationContextBackup;

public SynchronizationContextDeadlockTests()
{
synchronizationContextBackup = SynchronizationContext.Current;

syncContextMock = new Mock<SynchronizationContext>
{
CallBase = true
};

SynchronizationContext.SetSynchronizationContext(syncContextMock.Object);
}

[ClassCleanup]
public static void ClassCleanup()
{
SynchronizationContext.SetSynchronizationContext(synchronizationContextBackup);
}

[TestInitialize]
public void TestInitialize()
{
syncContextMock.Reset();
}

[TestMethod]
public void AutoPollDeadLockCheck()
{
var client = new ConfigCatClient(new AutoPollConfiguration
{
SdkKey = SDKKEY,
Logger = new ConsoleLogger(LogLevel.Off)
});

ClientDeadlockCheck(client);
}

[TestMethod]
public void ManualPollDeadLockCheck()
{
var client = new ConfigCatClient(new ManualPollConfiguration
{
SdkKey = SDKKEY,
Logger = new ConsoleLogger(LogLevel.Off)
});

ClientDeadlockCheck(client);
}

[TestMethod]
public void LazyLoadDeadLockCheck()
{
var client = new ConfigCatClient(new LazyLoadConfiguration
{
SdkKey = SDKKEY,
Logger = new ConsoleLogger(LogLevel.Off)
});

ClientDeadlockCheck(client);
}

private void ClientDeadlockCheck(IConfigCatClient client)
{
var methods = typeof(IConfigCatClient).GetMethods().Where(x => !x.IsSpecialName).OrderBy(o => o.Name);

foreach (var m in methods)
{
var parameters = Enumerable.Repeat<object>(null, m.GetParameters().Length).ToArray();

MethodInfo mi = m;

if (m.IsGenericMethod)
{
mi = m.MakeGenericMethod(typeof(string));
}

Console.WriteLine($"Invoke '{mi.Name}' method");

if (mi.ReturnType.IsSubclassOf(typeof(Task)))
{
var task = (Task)mi.Invoke(client, parameters);

task.ConfigureAwait(false);
task.Wait();
}
else
{
mi.Invoke(client, parameters);
}

syncContextMock.Verify(x => x.Post(It.IsAny<SendOrPostCallback>(), It.IsAny<object>()), Times.Never, $"Method: {mi.Name}");
syncContextMock.Verify(x => x.Send(It.IsAny<SendOrPostCallback>(), It.IsAny<object>()), Times.Never, $"Method: {mi.Name}");
}
}
}
}
52 changes: 37 additions & 15 deletions src/ConfigCatClient/ConfigCatClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@ public T GetValue<T>(string key, T defaultValue, User user = null)
{
try
{
var c = this.configService.GetConfigAsync().Result;
var config = this.configService.GetConfigAsync().GetAwaiter().GetResult();

return this.configEvaluator.Evaluate<T>(c, key, defaultValue, user);
return this.configEvaluator.Evaluate<T>(config, key, defaultValue, user);
}
catch (Exception ex)
{
Expand All @@ -160,9 +160,9 @@ public async Task<T> GetValueAsync<T>(string key, T defaultValue, User user = nu
{
try
{
var c = await this.configService.GetConfigAsync().ConfigureAwait(false);
var config = await this.configService.GetConfigAsync().ConfigureAwait(false);

return this.configEvaluator.Evaluate<T>(c, key, defaultValue, user);
return this.configEvaluator.Evaluate<T>(config, key, defaultValue, user);
}
catch (Exception ex)
{
Expand All @@ -177,7 +177,13 @@ public IEnumerable<string> GetAllKeys()
{
try
{
return this.GetAllKeysAsync().Result;
var config = this.configService.GetConfigAsync().GetAwaiter().GetResult();

if (this.configDeserializer.TryDeserialize(config, out var settings)) return settings.Keys;

this.log.Warning("Config deserialization failed.");

return new string[0];
}
catch (Exception ex)
{
Expand All @@ -191,10 +197,12 @@ public async Task<IEnumerable<string>> GetAllKeysAsync()
{
try
{
var c = await this.configService.GetConfigAsync().ConfigureAwait(false);
if (this.configDeserializer.TryDeserialize(c, out var settings)) return settings.Keys;
var config = await this.configService.GetConfigAsync().ConfigureAwait(false);

if (this.configDeserializer.TryDeserialize(config, out var settings)) return settings.Keys;

this.log.Warning("Config deserialization failed.");

return new string[0];
}
catch (Exception ex)
Expand All @@ -207,13 +215,27 @@ public async Task<IEnumerable<string>> GetAllKeysAsync()
/// <inheritdoc />
public void ForceRefresh()
{
this.configService.RefreshConfigAsync().Wait();
try
{
this.configService.RefreshConfigAsync().GetAwaiter().GetResult();
}
catch (Exception ex)
{
this.log.Error($"Error occured in 'ForceRefresh' method.\n{ex}");
}
}

/// <inheritdoc />
public async Task ForceRefreshAsync()
{
await this.configService.RefreshConfigAsync();
try
{
await this.configService.RefreshConfigAsync().ConfigureAwait(false);
}
catch (Exception ex)
{
this.log.Error($"Error occured in 'ForceRefreshAsync' method.\n{ex}");
}
}

/// <inheritdoc />
Expand All @@ -230,9 +252,9 @@ public string GetVariationId(string key, string defaultVariationId, User user =
{
try
{
var c = this.configService.GetConfigAsync().Result;
var config = this.configService.GetConfigAsync().GetAwaiter().GetResult();

return this.configEvaluator.EvaluateVariationId(c, key, defaultVariationId, user);
return this.configEvaluator.EvaluateVariationId(config, key, defaultVariationId, user);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -264,9 +286,9 @@ public IEnumerable<string> GetAllVariationId(User user = null)
{
try
{
var c = this.configService.GetConfigAsync().Result;
var config = this.configService.GetConfigAsync().GetAwaiter().GetResult();

return GetAllVariationIdLogic(c, user);
return GetAllVariationIdLogic(config, user);
}
catch (Exception ex)
{
Expand All @@ -281,9 +303,9 @@ public async Task<IEnumerable<string>> GetAllVariationIdAsync(User user = null)
{
try
{
var c = await this.configService.GetConfigAsync().ConfigureAwait(false);
var config = await this.configService.GetConfigAsync().ConfigureAwait(false);

return GetAllVariationIdLogic(c, user);
return GetAllVariationIdLogic(config, user);
}
catch (Exception ex)
{
Expand Down
4 changes: 2 additions & 2 deletions src/ConfigCatClient/ConfigService/AutoPollConfigService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public async Task<ProjectConfig> GetConfigAsync()
{
if (!initializedEventSlim.Wait(delay))
{
await RefreshLogicAsync("init");
await RefreshLogicAsync("init").ConfigureAwait(false);
}

cacheConfig = await this.configCache.GetAsync(base.cacheKey).ConfigureAwait(false);
Expand All @@ -78,7 +78,7 @@ public async Task<ProjectConfig> GetConfigAsync()

public async Task RefreshConfigAsync()
{
await RefreshLogicAsync("manual");
await RefreshLogicAsync("manual").ConfigureAwait(false);
}

private async Task RefreshLogicAsync(object sender)
Expand Down
2 changes: 1 addition & 1 deletion src/ConfigCatClient/HttpConfigFetcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public async Task<ProjectConfig> Fetch(ProjectConfig lastConfig)

try
{
var fetchResult = await FetchRequest(lastConfig, this.requestUri);
var fetchResult = await FetchRequest(lastConfig, this.requestUri).ConfigureAwait(false);

var response = fetchResult.Item1;

Expand Down

0 comments on commit 6b7b24c

Please sign in to comment.