Skip to content

Commit

Permalink
Ensure correct thread for AvaloniaProperty access.
Browse files Browse the repository at this point in the history 
Previously we ensured that `AvaloniaObject.SetValue` was run on the main
thread. This makes sure that `GetValue`, `IsSet` and `ClearValue` are
also run on the main thread. Unit testing this turned out to be more
complicated than expected, because `Dispatcher` keeps a hold of a
reference to the first `IPlatformThreadingInterface` it sees, so made
`UnitTestApplication` able to notify `Dispatcher` that it should update
its services.
  • Loading branch information
@grokys
grokys committed Mar 5, 2017
1 parent b953d3f commit a46be4e
Show file tree
Hide file tree
Showing 14 changed files with 325 additions and 32 deletions.
2 changes: 1 addition & 1 deletion Avalonia.v3.ncrunchsolution
View file
Expand Up @@ -3,7 +3,7 @@
<AdditionalFilesToIncludeForSolution>
<Value>tests\TestFiles\**.*</Value>
</AdditionalFilesToIncludeForSolution>
<AllowParallelTestExecution>False</AllowParallelTestExecution>
<AllowParallelTestExecution>True</AllowParallelTestExecution>
<ProjectConfigStoragePathRelativeToSolutionDir>.ncrunch</ProjectConfigStoragePathRelativeToSolutionDir>
<SolutionConfigured>True</SolutionConfigured>
</Settings>
Expand Down
6 changes: 5 additions & 1 deletion src/Avalonia.Base/AvaloniaObject.cs
View file
Expand Up @@ -181,6 +181,7 @@ select new
public void ClearValue(AvaloniaProperty property)
{
Contract.Requires<ArgumentNullException>(property != null);
VerifyAccess();

SetValue(property, AvaloniaProperty.UnsetValue);
}
Expand All @@ -193,6 +194,7 @@ public void ClearValue(AvaloniaProperty property)
public object GetValue(AvaloniaProperty property)
{
Contract.Requires<ArgumentNullException>(property != null);
VerifyAccess();

if (property.IsDirect)
{
Expand Down Expand Up @@ -234,7 +236,8 @@ public T GetValue<T>(AvaloniaProperty<T> property)
public bool IsSet(AvaloniaProperty property)
{
Contract.Requires<ArgumentNullException>(property != null);

VerifyAccess();

PriorityValue value;

if (_values.TryGetValue(property, out value))
Expand Down Expand Up @@ -332,6 +335,7 @@ public bool IsSet(AvaloniaProperty property)
}

subscription = source
.Do(_ => VerifyAccess())
.Select(x => CastOrDefault(x, property.PropertyType))
.Do(_ => { }, () => _directBindings.Remove(subscription))
.Subscribe(x => SetDirectValue(property, x));
Expand Down
5 changes: 5 additions & 0 deletions src/Avalonia.Base/IPriorityValueOwner.cs
View file
Expand Up @@ -25,5 +25,10 @@ internal interface IPriorityValueOwner
/// <param name="sender">The source of the change.</param>
/// <param name="notification">The notification.</param>
void BindingNotificationReceived(PriorityValue sender, BindingNotification notification);

/// <summary>
/// Ensures that the current thread is the UI thread.
/// </summary>
void VerifyAccess();
}
}
2 changes: 2 additions & 0 deletions src/Avalonia.Base/PriorityBindingEntry.cs
View file
Expand Up @@ -92,6 +92,8 @@ public void Dispose()

private void ValueChanged(object value)
{
_owner.Owner.Owner?.VerifyAccess();

var notification = value as BindingNotification;

if (notification != null)
Expand Down
18 changes: 11 additions & 7 deletions src/Avalonia.Base/PriorityLevel.cs
View file
Expand Up @@ -33,7 +33,6 @@ namespace Avalonia
/// </remarks>
internal class PriorityLevel
{
private PriorityValue _owner;
private object _directValue;
private int _nextIndex;

Expand All @@ -48,13 +47,18 @@ internal class PriorityLevel
{
Contract.Requires<ArgumentNullException>(owner != null);

_owner = owner;
Owner = owner;
Priority = priority;
Value = _directValue = AvaloniaProperty.UnsetValue;
ActiveBindingIndex = -1;
Bindings = new LinkedList<PriorityBindingEntry>();
}

/// <summary>
/// Gets the owner of the level.
/// </summary>
public PriorityValue Owner { get; }

/// <summary>
/// Gets the priority of this level.
/// </summary>
Expand All @@ -73,7 +77,7 @@ public object DirectValue
set
{
Value = _directValue = value;
_owner.LevelValueChanged(this);
Owner.LevelValueChanged(this);
}
}

Expand Down Expand Up @@ -131,7 +135,7 @@ public void Changed(PriorityBindingEntry entry)
{
Value = entry.Value;
ActiveBindingIndex = entry.Index;
_owner.LevelValueChanged(this);
Owner.LevelValueChanged(this);
}
else
{
Expand Down Expand Up @@ -161,7 +165,7 @@ public void Completed(PriorityBindingEntry entry)
/// <param name="error">The error.</param>
public void Error(PriorityBindingEntry entry, BindingNotification error)
{
_owner.LevelError(this, error);
Owner.LevelError(this, error);
}

/// <summary>
Expand All @@ -175,14 +179,14 @@ private void ActivateFirstBinding()
{
Value = binding.Value;
ActiveBindingIndex = binding.Index;
_owner.LevelValueChanged(this);
Owner.LevelValueChanged(this);
return;
}
}

Value = DirectValue;
ActiveBindingIndex = -1;
_owner.LevelValueChanged(this);
Owner.LevelValueChanged(this);
}
}
}
18 changes: 11 additions & 7 deletions src/Avalonia.Base/PriorityValue.cs
View file
Expand Up @@ -26,7 +26,6 @@ namespace Avalonia
/// </remarks>
internal class PriorityValue
{
private readonly IPriorityValueOwner _owner;
private readonly Type _valueType;
private readonly SingleOrDictionary<int, PriorityLevel> _levels = new SingleOrDictionary<int, PriorityLevel>();
private object _value;
Expand All @@ -45,14 +44,19 @@ internal class PriorityValue
Type valueType,
Func<object, object> validate = null)
{
_owner = owner;
Owner = owner;
Property = property;
_valueType = valueType;
_value = AvaloniaProperty.UnsetValue;
ValuePriority = int.MaxValue;
_validate = validate;
}

/// <summary>
/// Gets the owner of the value.
/// </summary>
public IPriorityValueOwner Owner { get; }

/// <summary>
/// Gets the property that the value represents.
/// </summary>
Expand Down Expand Up @@ -188,9 +192,9 @@ public void LevelError(PriorityLevel level, BindingNotification error)
Logger.Log(
LogEventLevel.Error,
LogArea.Binding,
_owner,
Owner,
"Error in binding to {Target}.{Property}: {Message}",
_owner,
Owner,
Property,
error.Error.Message);
}
Expand Down Expand Up @@ -264,19 +268,19 @@ private void UpdateValue(object value, int priority)

if (notification == null || notification.HasValue)
{
_owner?.Changed(this, old, _value);
Owner?.Changed(this, old, _value);
}

if (notification != null)
{
_owner?.BindingNotificationReceived(this, notification);
Owner?.BindingNotificationReceived(this, notification);
}
}
else
{
Logger.Error(
LogArea.Binding,
_owner,
Owner,
"Binding produced invalid value for {$Property} ({$PropertyType}): {$Value} ({$ValueType})",
Property.Name,
_valueType,
Expand Down
1 change: 1 addition & 0 deletions src/Avalonia.Base/Properties/AssemblyInfo.cs
View file
Expand Up @@ -6,4 +6,5 @@

[assembly: AssemblyTitle("Avalonia.Base")]
[assembly: InternalsVisibleTo("Avalonia.Base.UnitTests")]
[assembly: InternalsVisibleTo("Avalonia.UnitTests")]
[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")]
40 changes: 34 additions & 6 deletions src/Avalonia.Base/Threading/Dispatcher.cs
View file
Expand Up @@ -17,31 +17,40 @@ namespace Avalonia.Threading
/// </remarks>
public class Dispatcher
{
private readonly IPlatformThreadingInterface _platform;
private readonly JobRunner _jobRunner;
private IPlatformThreadingInterface _platform;

public static Dispatcher UIThread { get; } =
new Dispatcher(AvaloniaLocator.Current.GetService<IPlatformThreadingInterface>());

public Dispatcher(IPlatformThreadingInterface platform)
{
_platform = platform;
if(_platform == null)
//TODO: Unit test mode, fix that somehow
return;
_jobRunner = new JobRunner(platform);
_platform.Signaled += _jobRunner.RunJobs;

if (_platform != null)
{
_platform.Signaled += _jobRunner.RunJobs;
}
}

/// <summary>
/// Checks that the current thread is the UI thread.
/// </summary>
public bool CheckAccess() => _platform?.CurrentThreadIsLoopThread ?? true;

/// <summary>
/// Checks that the current thread is the UI thread and throws if not.
/// </summary>
/// <exception cref="InvalidOperationException">
/// The current thread is not the UI thread.
/// </exception>
public void VerifyAccess()
{
if (!CheckAccess())
throw new InvalidOperationException("Call from invalid thread");
}


/// <summary>
/// Runs the dispatcher's main loop.
/// </summary>
Expand Down Expand Up @@ -83,5 +92,24 @@ public void InvokeAsync(Action action, DispatcherPriority priority = DispatcherP
{
_jobRunner?.Post(action, priority);
}

/// <summary>
/// Allows unit tests to change the platform threading interface.
/// </summary>
internal void UpdateServices()
{
if (_platform != null)
{
_platform.Signaled -= _jobRunner.RunJobs;
}

_platform = AvaloniaLocator.Current.GetService<IPlatformThreadingInterface>();
_jobRunner.UpdateServices();

if (_platform != null)
{
_platform.Signaled += _jobRunner.RunJobs;
}
}
}
}
13 changes: 10 additions & 3 deletions src/Avalonia.Base/Threading/JobRunner.cs
View file
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Avalonia.Platform;

Expand All @@ -14,8 +13,8 @@ namespace Avalonia.Threading
/// </summary>
internal class JobRunner
{
private readonly IPlatformThreadingInterface _platform;
private readonly Queue<Job> _queue = new Queue<Job>();
private IPlatformThreadingInterface _platform;

public JobRunner(IPlatformThreadingInterface platform)
{
Expand Down Expand Up @@ -82,6 +81,14 @@ internal void Post(Action action, DispatcherPriority priority)
AddJob(new Job(action, priority, true));
}

/// <summary>
/// Allows unit tests to change the platform threading interface.
/// </summary>
internal void UpdateServices()
{
_platform = AvaloniaLocator.Current.GetService<IPlatformThreadingInterface>();
}

private void AddJob(Job job)
{
var needWake = false;
Expand All @@ -91,7 +98,7 @@ private void AddJob(Job job)
_queue.Enqueue(job);
}
if (needWake)
_platform.Signal();
_platform?.Signal();
}

/// <summary>
Expand Down
21 changes: 21 additions & 0 deletions tests/Avalonia.Base.UnitTests/Avalonia.Base.UnitTests.csproj
View file
Expand Up @@ -90,6 +90,7 @@
<Otherwise />
</Choose>
<ItemGroup>
<Compile Include="AvaloniaObjectTests_Threading.cs" />
<Compile Include="AvaloniaObjectTests_DataValidation.cs" />
<Compile Include="Collections\CollectionChangedTracker.cs" />
<Compile Include="Collections\AvaloniaDictionaryTests.cs" />
Expand Down Expand Up @@ -124,6 +125,26 @@
<Project>{B09B78D8-9B26-48B0-9149-D64A2F120F3F}</Project>
<Name>Avalonia.Base</Name>
</ProjectReference>
<ProjectReference Include="..\..\src\Avalonia.Controls\Avalonia.Controls.csproj">
<Project>{d2221c82-4a25-4583-9b43-d791e3f6820c}</Project>
<Name>Avalonia.Controls</Name>
</ProjectReference>
<ProjectReference Include="..\..\src\Avalonia.Input\Avalonia.Input.csproj">
<Project>{62024b2d-53eb-4638-b26b-85eeaa54866e}</Project>
<Name>Avalonia.Input</Name>
</ProjectReference>
<ProjectReference Include="..\..\src\Avalonia.Layout\Avalonia.Layout.csproj">
<Project>{42472427-4774-4c81-8aff-9f27b8e31721}</Project>
<Name>Avalonia.Layout</Name>
</ProjectReference>
<ProjectReference Include="..\..\src\Avalonia.Styling\Avalonia.Styling.csproj">
<Project>{f1baa01a-f176-4c6a-b39d-5b40bb1b148f}</Project>
<Name>Avalonia.Styling</Name>
</ProjectReference>
<ProjectReference Include="..\..\src\Avalonia.Visuals\Avalonia.Visuals.csproj">
<Project>{eb582467-6abb-43a1-b052-e981ba910e3a}</Project>
<Name>Avalonia.Visuals</Name>
</ProjectReference>
<ProjectReference Include="..\Avalonia.UnitTests\Avalonia.UnitTests.csproj">
<Project>{88060192-33d5-4932-b0f9-8bd2763e857d}</Project>
<Name>Avalonia.UnitTests</Name>
Expand Down
12 changes: 6 additions & 6 deletions tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs
View file
Expand Up @@ -365,7 +365,7 @@ public void Bind_Logs_Binding_Error()
}

[Fact]
public async void Bind_With_Scheduler_Executes_On_Scheduler()
public async Task Bind_With_Scheduler_Executes_On_Scheduler()
{
var target = new Class1();
var source = new Subject<object>();
Expand All @@ -375,16 +375,16 @@ public async void Bind_With_Scheduler_Executes_On_Scheduler()
threadingInterfaceMock.SetupGet(mock => mock.CurrentThreadIsLoopThread)
.Returns(() => Thread.CurrentThread.ManagedThreadId == currentThreadId);

using (AvaloniaLocator.EnterScope())
{
AvaloniaLocator.CurrentMutable.Bind<IPlatformThreadingInterface>().ToConstant(threadingInterfaceMock.Object);
AvaloniaLocator.CurrentMutable.Bind<IScheduler>().ToConstant(AvaloniaScheduler.Instance);
var services = new TestServices(
scheduler: AvaloniaScheduler.Instance,
threadingInterface: threadingInterfaceMock.Object);

using (UnitTestApplication.Start(services))
{
target.Bind(Class1.QuxProperty, source);

await Task.Run(() => source.OnNext(6.7));
}

}

/// <summary>
Expand Down

0 comments on commit a46be4e

Please sign in to comment.