Merge pull request #909 from AvaloniaUI/avalonia-object-thread-affinity

Ensure correct thread for AvaloniaProperty access.

Avalonia.v3.ncrunchsolution

@@ -3,7 +3,7 @@
     <AdditionalFilesToIncludeForSolution>
       <Value>tests\TestFiles\**.*</Value>
     </AdditionalFilesToIncludeForSolution>
-    <AllowParallelTestExecution>False</AllowParallelTestExecution>
+    <AllowParallelTestExecution>True</AllowParallelTestExecution>
     <ProjectConfigStoragePathRelativeToSolutionDir>.ncrunch</ProjectConfigStoragePathRelativeToSolutionDir>
     <SolutionConfigured>True</SolutionConfigured>
   </Settings>

src/Avalonia.Base/AvaloniaObject.cs

@@ -181,6 +181,7 @@ namespace Avalonia
         public void ClearValue(AvaloniaProperty property)
         {
             Contract.Requires<ArgumentNullException>(property != null);
+            VerifyAccess();
 
             SetValue(property, AvaloniaProperty.UnsetValue);
         }
@@ -193,6 +194,7 @@ namespace Avalonia
         public object GetValue(AvaloniaProperty property)
         {
             Contract.Requires<ArgumentNullException>(property != null);
+            VerifyAccess();
 
             if (property.IsDirect)
             {
@@ -234,7 +236,8 @@ namespace Avalonia
         public bool IsSet(AvaloniaProperty property)
         {
             Contract.Requires<ArgumentNullException>(property != null);
-            
+            VerifyAccess();
+
             PriorityValue value;
 
             if (_values.TryGetValue(property, out value))

src/Avalonia.Base/IPriorityValueOwner.cs

@@ -25,5 +25,10 @@ namespace Avalonia
         /// <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();
     }
 }

src/Avalonia.Base/PriorityBindingEntry.cs

@@ -92,6 +92,8 @@ namespace Avalonia
 
         private void ValueChanged(object value)
         {
+            _owner.Owner.Owner?.VerifyAccess();
+
             var notification = value as BindingNotification;
 
             if (notification != null)

src/Avalonia.Base/PriorityLevel.cs

@@ -33,7 +33,6 @@ namespace Avalonia
     /// </remarks>
     internal class PriorityLevel
     {
-        private PriorityValue _owner;
         private object _directValue;
         private int _nextIndex;
 
@@ -48,13 +47,18 @@ namespace Avalonia
         {
             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>
@@ -73,7 +77,7 @@ namespace Avalonia
             set
             {
                 Value = _directValue = value;
-                _owner.LevelValueChanged(this);
+                Owner.LevelValueChanged(this);
             }
         }
 
@@ -131,7 +135,7 @@ namespace Avalonia
                 {
                     Value = entry.Value;
                     ActiveBindingIndex = entry.Index;
-                    _owner.LevelValueChanged(this);
+                    Owner.LevelValueChanged(this);
                 }
                 else
                 {
@@ -161,7 +165,7 @@ namespace Avalonia
         /// <param name="error">The error.</param>
         public void Error(PriorityBindingEntry entry, BindingNotification error)
         {
-            _owner.LevelError(this, error);
+            Owner.LevelError(this, error);
         }
 
         /// <summary>
@@ -175,14 +179,14 @@ namespace Avalonia
                 {
                     Value = binding.Value;
                     ActiveBindingIndex = binding.Index;
-                    _owner.LevelValueChanged(this);
+                    Owner.LevelValueChanged(this);
                     return;
                 }
             }
 
             Value = DirectValue;
             ActiveBindingIndex = -1;
-            _owner.LevelValueChanged(this);
+            Owner.LevelValueChanged(this);
         }
     }
 }

src/Avalonia.Base/PriorityValue.cs

@@ -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;
@@ -45,7 +44,7 @@ namespace Avalonia
             Type valueType,
             Func<object, object> validate = null)
         {
-            _owner = owner;
+            Owner = owner;
             Property = property;
             _valueType = valueType;
             _value = AvaloniaProperty.UnsetValue;
@@ -53,6 +52,11 @@ namespace Avalonia
             _validate = validate;
         }
 
+        /// <summary>
+        /// Gets the owner of the value.
+        /// </summary>
+        public IPriorityValueOwner Owner { get; }
+
         /// <summary>
         /// Gets the property that the value represents.
         /// </summary>
@@ -188,9 +192,9 @@ namespace Avalonia
             Logger.Log(
                 LogEventLevel.Error,
                 LogArea.Binding,
-                _owner,
+                Owner,
                 "Error in binding to {Target}.{Property}: {Message}",
-                _owner,
+                Owner,
                 Property,
                 error.Error.Message);
         }
@@ -264,19 +268,19 @@ namespace Avalonia
 
                 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, 

src/Avalonia.Base/Properties/AssemblyInfo.cs

@@ -6,4 +6,5 @@ using System.Runtime.CompilerServices;
 
 [assembly: AssemblyTitle("Avalonia.Base")]
 [assembly: InternalsVisibleTo("Avalonia.Base.UnitTests")]
+[assembly: InternalsVisibleTo("Avalonia.UnitTests")]
 [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")] 

src/Avalonia.Base/Threading/Dispatcher.cs

@@ -17,8 +17,8 @@ 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>());
@@ -26,22 +26,31 @@ namespace Avalonia.Threading
         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>
@@ -83,5 +92,24 @@ namespace Avalonia.Threading
         {
             _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;
+            }
+        }
     }
 }

src/Avalonia.Base/Threading/JobRunner.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Threading;
 using System.Threading.Tasks;
 using Avalonia.Platform;
 
@@ -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)
         {
@@ -82,6 +81,14 @@ namespace Avalonia.Threading
             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;
@@ -91,7 +98,7 @@ namespace Avalonia.Threading
                 _queue.Enqueue(job);
             }
             if (needWake)
-                _platform.Signal();
+                _platform?.Signal();
         }
 
         /// <summary>

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Binding.cs

@@ -365,7 +365,7 @@ namespace Avalonia.Base.UnitTests
         }
         
         [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>();
@@ -375,16 +375,16 @@ namespace Avalonia.Base.UnitTests
             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>

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Threading.cs

@@ -0,0 +1,168 @@
+// Copyright (c) The Avalonia Project. All rights reserved.
+// Licensed under the MIT license. See licence.md file in the project root for full license information.
+
+using System;
+using System.Reactive.Subjects;
+using System.Threading;
+using System.Threading.Tasks;
+using Avalonia.Platform;
+using Avalonia.UnitTests;
+using Xunit;
+
+namespace Avalonia.Base.UnitTests
+{
+    public class AvaloniaObjectTests_Threading
+    {
+        [Fact]
+        public void StyledProperty_GetValue_Should_Throw()
+        {
+            using (UnitTestApplication.Start(new TestServices(threadingInterface: new ThreadingInterface())))
+            {
+                var target = new Class1();
+
+                Assert.Throws<InvalidOperationException>(() => target.GetValue(Class1.StyledProperty));
+            }
+        }
+
+        [Fact]
+        public void StyledProperty_SetValue_Should_Throw()
+        {
+            using (UnitTestApplication.Start(new TestServices(threadingInterface: new ThreadingInterface())))
+            {
+                var target = new Class1();
+
+                Assert.Throws<InvalidOperationException>(() => target.SetValue(Class1.StyledProperty, "foo"));
+            }
+        }
+
+        [Fact]
+        public void Setting_StyledProperty_Binding_Should_Throw()
+        {
+            using (UnitTestApplication.Start(new TestServices(threadingInterface: new ThreadingInterface())))
+            {
+                var target = new Class1();
+
+                Assert.Throws<InvalidOperationException>(() => 
+                    target.Bind(
+                        Class1.StyledProperty,
+                        new BehaviorSubject<string>("foo")));
+            }
+        }
+
+        [Fact]
+        public void StyledProperty_ClearValue_Should_Throw()
+        {
+            using (UnitTestApplication.Start(new TestServices(threadingInterface: new ThreadingInterface())))
+            {
+                var target = new Class1();
+
+                Assert.Throws<InvalidOperationException>(() => target.ClearValue(Class1.StyledProperty));
+            }
+        }
+
+        [Fact]
+        public void StyledProperty_IsSet_Should_Throw()
+        {
+            using (UnitTestApplication.Start(new TestServices(threadingInterface: new ThreadingInterface())))
+            {
+                var target = new Class1();
+
+                Assert.Throws<InvalidOperationException>(() => target.IsSet(Class1.StyledProperty));
+            }
+        }
+
+        [Fact]
+        public void DirectProperty_GetValue_Should_Throw()
+        {
+            using (UnitTestApplication.Start(new TestServices(threadingInterface: new ThreadingInterface())))
+            {
+                var target = new Class1();
+
+                Assert.Throws<InvalidOperationException>(() => target.GetValue(Class1.DirectProperty));
+            }
+        }
+
+        [Fact]
+        public void DirectProperty_SetValue_Should_Throw()
+        {
+            using (UnitTestApplication.Start(new TestServices(threadingInterface: new ThreadingInterface())))
+            {
+                var target = new Class1();
+
+                Assert.Throws<InvalidOperationException>(() => target.SetValue(Class1.DirectProperty, "foo"));
+            }
+        }
+
+        [Fact]
+        public void Setting_DirectProperty_Binding_Should_Throw()
+        {
+            using (UnitTestApplication.Start(new TestServices(threadingInterface: new ThreadingInterface())))
+            {
+                var target = new Class1();
+
+                Assert.Throws<InvalidOperationException>(() =>
+                    target.Bind(
+                        Class1.DirectProperty,
+                        new BehaviorSubject<string>("foo")));
+            }
+        }
+
+        [Fact]
+        public void DirectProperty_ClearValue_Should_Throw()
+        {
+            using (UnitTestApplication.Start(new TestServices(threadingInterface: new ThreadingInterface())))
+            {
+                var target = new Class1();
+
+                Assert.Throws<InvalidOperationException>(() => target.ClearValue(Class1.DirectProperty));
+            }
+        }
+
+        [Fact]
+        public void DirectProperty_IsSet_Should_Throw()
+        {
+            using (UnitTestApplication.Start(new TestServices(threadingInterface: new ThreadingInterface())))
+            {
+                var target = new Class1();
+
+                Assert.Throws<InvalidOperationException>(() => target.IsSet(Class1.DirectProperty));
+            }
+        }
+
+        private class Class1 : AvaloniaObject
+        {
+            public static readonly StyledProperty<string> StyledProperty =
+                AvaloniaProperty.Register<Class1, string>("Foo", "foodefault");
+
+            public static readonly DirectProperty<Class1, string> DirectProperty =
+                AvaloniaProperty.RegisterDirect<Class1, string>("Qux", _ => null, (o, v) => { });
+        }
+
+        private class ThreadingInterface : IPlatformThreadingInterface
+        {
+            public ThreadingInterface(bool isLoopThread = false)
+            {
+                CurrentThreadIsLoopThread = isLoopThread;
+            }
+
+            public bool CurrentThreadIsLoopThread { get; set; }
+
+            public event Action Signaled;
+
+            public void RunLoop(CancellationToken cancellationToken)
+            {
+                throw new NotImplementedException();
+            }
+
+            public void Signal()
+            {
+                throw new NotImplementedException();
+            }
+
+            public IDisposable StartTimer(TimeSpan interval, Action tick)
+            {
+                throw new NotImplementedException();
+            }
+        }
+    }
+}

tests/Avalonia.UnitTests/TestServices.cs

@@ -12,6 +12,7 @@ using Avalonia.Shared.PlatformSupport;
 using Avalonia.Styling;
 using Avalonia.Themes.Default;
 using Avalonia.Rendering;
+using System.Reactive.Concurrency;
 
 namespace Avalonia.UnitTests
 {
@@ -63,6 +64,7 @@ namespace Avalonia.UnitTests
             IRenderer renderer = null,
             IPlatformRenderInterface renderInterface = null,
             IRenderLoop renderLoop = null,
+            IScheduler scheduler = null,
             IStandardCursorFactory standardCursorFactory = null,
             IStyler styler = null,
             Func<Styles> theme = null,
@@ -78,6 +80,7 @@ namespace Avalonia.UnitTests
             Renderer = renderer;
             RenderInterface = renderInterface;
             RenderLoop = renderLoop;
+            Scheduler = scheduler;
             StandardCursorFactory = standardCursorFactory;
             Styler = styler;
             Theme = theme;
@@ -94,6 +97,7 @@ namespace Avalonia.UnitTests
         public IRenderer Renderer { get; }
         public IPlatformRenderInterface RenderInterface { get; }
         public IRenderLoop RenderLoop { get; }
+        public IScheduler Scheduler { get; }
         public IStandardCursorFactory StandardCursorFactory { get; }
         public IStyler Styler { get; }
         public Func<Styles> Theme { get; }
@@ -110,6 +114,7 @@ namespace Avalonia.UnitTests
             IRenderer renderer = null,
             IPlatformRenderInterface renderInterface = null,
             IRenderLoop renderLoop = null,
+            IScheduler scheduler = null,
             IStandardCursorFactory standardCursorFactory = null,
             IStyler styler = null,
             Func<Styles> theme = null,
@@ -127,6 +132,7 @@ namespace Avalonia.UnitTests
                 renderer: renderer ?? Renderer,
                 renderInterface: renderInterface ?? RenderInterface,
                 renderLoop: renderLoop ?? RenderLoop,
+                scheduler: scheduler ?? Scheduler,
                 standardCursorFactory: standardCursorFactory ?? StandardCursorFactory,
                 styler: styler ?? Styler,
                 theme: theme ?? Theme,

tests/Avalonia.UnitTests/UnitTestApplication.cs

@@ -8,6 +8,9 @@ using Avalonia.Platform;
 using Avalonia.Styling;
 using Avalonia.Controls;
 using Avalonia.Rendering;
+using Avalonia.Threading;
+using System.Reactive.Disposables;
+using System.Reactive.Concurrency;
 
 namespace Avalonia.UnitTests
 {
@@ -30,7 +33,12 @@ namespace Avalonia.UnitTests
             var scope = AvaloniaLocator.EnterScope();
             var app = new UnitTestApplication(services);
             AvaloniaLocator.CurrentMutable.BindToSelf<Application>(app);
-            return scope;
+            Dispatcher.UIThread.UpdateServices();
+            return Disposable.Create(() =>
+            {
+                scope.Dispose();
+                Dispatcher.UIThread.UpdateServices();
+            });
         }
 
         public override void RegisterServices()
@@ -47,6 +55,7 @@ namespace Avalonia.UnitTests
                 .Bind<IPlatformRenderInterface>().ToConstant(Services.RenderInterface)
                 .Bind<IRenderLoop>().ToConstant(Services.RenderLoop)
                 .Bind<IPlatformThreadingInterface>().ToConstant(Services.ThreadingInterface)
+                .Bind<IScheduler>().ToConstant(Services.Scheduler)
                 .Bind<IStandardCursorFactory>().ToConstant(Services.StandardCursorFactory)
                 .Bind<IStyler>().ToConstant(Services.Styler)
                 .Bind<IWindowingPlatform>().ToConstant(Services.WindowingPlatform)