From 324cdafdbdee2c03adce82b08ccfaf5df1f81eec Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Fri, 22 Sep 2023 03:15:58 +0300 Subject: [PATCH] Added guards for compositor reentrancy and exposed batch lifetime events (#12968) * Added guards for compositor reentrancy and exposed batch lifetime events * Use manual continue with since our tests are using some borked sync context --- .../Media/MediaContext.Compositor.cs | 4 +- src/Avalonia.Base/Media/MediaContext.cs | 2 +- .../Composition/CompositingRenderer.cs | 20 +++++--- .../Rendering/Composition/Compositor.cs | 20 +++++--- .../Composition/Server/ServerCompositor.cs | 46 +++++++++++++++---- .../Composition/Server/SimpleServerObject.cs | 2 +- .../Rendering/Composition/Transport/Batch.cs | 36 ++++++++++++--- .../Threading/Dispatcher.Invoke.cs | 12 +++++ .../Threading/DispatcherPriorityAwaitable.cs | 40 ++++++++++++++++ 9 files changed, 148 insertions(+), 34 deletions(-) create mode 100644 src/Avalonia.Base/Threading/DispatcherPriorityAwaitable.cs diff --git a/src/Avalonia.Base/Media/MediaContext.Compositor.cs b/src/Avalonia.Base/Media/MediaContext.Compositor.cs index 4ddc2ea9eb..1c4c18838f 100644 --- a/src/Avalonia.Base/Media/MediaContext.Compositor.cs +++ b/src/Avalonia.Base/Media/MediaContext.Compositor.cs @@ -16,7 +16,7 @@ partial class MediaContext /// Actually sends the current batch to the compositor and does the required housekeeping /// This is the only place that should be allowed to call Commit /// - private Batch CommitCompositor(Compositor compositor) + private CompositionBatch CommitCompositor(Compositor compositor) { var commit = compositor.Commit(); _requestedCommits.Remove(compositor); @@ -29,7 +29,7 @@ partial class MediaContext /// /// Handles batch completion, required to re-schedule a render pass if one was skipped due to compositor throttling /// - private void CompositionBatchFinished(Compositor compositor, Batch batch) + private void CompositionBatchFinished(Compositor compositor, CompositionBatch batch) { // Check if it was the last commited batch, since sometimes we are forced to send a new // one without waiting for the previous one to complete diff --git a/src/Avalonia.Base/Media/MediaContext.cs b/src/Avalonia.Base/Media/MediaContext.cs index 0a290052c7..af32406f03 100644 --- a/src/Avalonia.Base/Media/MediaContext.cs +++ b/src/Avalonia.Base/Media/MediaContext.cs @@ -22,7 +22,7 @@ internal partial class MediaContext : ICompositorScheduler private readonly Action _render; private readonly Action _inputMarkerHandler; private readonly HashSet _requestedCommits = new(); - private readonly Dictionary _pendingCompositionBatches = new(); + private readonly Dictionary _pendingCompositionBatches = new(); private record TopLevelInfo(Compositor Compositor, CompositingRenderer Renderer, ILayoutManager LayoutManager); private List? _invokeOnRenderCallbacks; diff --git a/src/Avalonia.Base/Rendering/Composition/CompositingRenderer.cs b/src/Avalonia.Base/Rendering/Composition/CompositingRenderer.cs index 4db14ba183..ca00b94eaf 100644 --- a/src/Avalonia.Base/Rendering/Composition/CompositingRenderer.cs +++ b/src/Avalonia.Base/Rendering/Composition/CompositingRenderer.cs @@ -8,6 +8,7 @@ using Avalonia.Collections; using Avalonia.Collections.Pooled; using Avalonia.Media; using Avalonia.Rendering.Composition.Drawing; +using Avalonia.Threading; using Avalonia.VisualTree; namespace Avalonia.Rendering.Composition; @@ -25,6 +26,7 @@ internal class CompositingRenderer : IRendererWithCompositor, IHitTester private readonly Action _update; private bool _queuedUpdate; + private bool _queuedSceneInvalidation; private bool _updating; private bool _isDisposed; @@ -172,13 +174,17 @@ internal class CompositingRenderer : IRendererWithCompositor, IHitTester _recalculateChildren.Clear(); CompositionTarget.Size = _root.ClientSize; CompositionTarget.Scaling = _root.RenderScaling; - TriggerSceneInvalidatedOnBatchCompletion(_compositor.RequestCommitAsync()); - } - - private async void TriggerSceneInvalidatedOnBatchCompletion(Task batchCompletion) - { - await batchCompletion; - SceneInvalidated?.Invoke(this, new SceneInvalidatedEventArgs(_root, new Rect(_root.ClientSize))); + + var commit = _compositor.RequestCommitAsync(); + if (!_queuedSceneInvalidation) + { + _queuedSceneInvalidation = true; + commit.ContinueWith(_ => Dispatcher.UIThread.Post(() => + { + _queuedSceneInvalidation = false; + SceneInvalidated?.Invoke(this, new SceneInvalidatedEventArgs(_root, new Rect(_root.ClientSize))); + }, DispatcherPriority.Input)); + } } public void TriggerSceneInvalidatedForUnitTests(Rect rect) => diff --git a/src/Avalonia.Base/Rendering/Composition/Compositor.cs b/src/Avalonia.Base/Rendering/Composition/Compositor.cs index 902a195098..84bc7795f2 100644 --- a/src/Avalonia.Base/Rendering/Composition/Compositor.cs +++ b/src/Avalonia.Base/Rendering/Composition/Compositor.cs @@ -24,7 +24,7 @@ namespace Avalonia.Rendering.Composition internal IRenderLoop Loop { get; } internal bool UseUiThreadForSynchronousCommits { get; } private ServerCompositor _server; - private Batch? _nextCommit; + private CompositionBatch? _nextCommit; private BatchStreamObjectPool _batchObjectPool; private BatchStreamMemoryPool _batchMemoryPool; private Queue _objectSerializationQueue = new(); @@ -32,7 +32,7 @@ namespace Avalonia.Rendering.Composition private Queue _invokeBeforeCommitWrite = new(), _invokeBeforeCommitRead = new(); private HashSet _disposeOnNextBatch = new(); internal ServerCompositor Server => _server; - private Batch? _pendingBatch; + private CompositionBatch? _pendingBatch; private readonly object _pendingBatchLock = new(); private List _pendingServerCompositorJobs = new(); private DiagnosticTextRenderer? _diagnosticTextRenderer; @@ -90,8 +90,14 @@ namespace Avalonia.Rendering.Composition /// /// Requests pending changes in the composition objects to be serialized and sent to the render thread /// - /// A task that completes when sent changes are applied and rendered on the render thread - public Task RequestCommitAsync() + /// A task that completes when sent changes are applied on the render thread + public Task RequestCommitAsync() => RequestCompositionBatchCommitAsync().Processed; + + /// + /// Requests pending changes in the composition objects to be serialized and sent to the render thread + /// + /// A CompositionBatch object that provides batch lifetime information + public CompositionBatch RequestCompositionBatchCommitAsync() { Dispatcher.UIThread.VerifyAccess(); if (_nextCommit == null) @@ -105,10 +111,10 @@ namespace Avalonia.Rendering.Composition _triggerCommitRequested(); } - return _nextCommit.Processed; + return _nextCommit; } - internal Batch Commit() + internal CompositionBatch Commit() { try { @@ -122,7 +128,7 @@ namespace Avalonia.Rendering.Composition } } - Batch CommitCore() + CompositionBatch CommitCore() { Dispatcher.UIThread.VerifyAccess(); using var noPump = NonPumpingLockHelper.Use(); diff --git a/src/Avalonia.Base/Rendering/Composition/Server/ServerCompositor.cs b/src/Avalonia.Base/Rendering/Composition/Server/ServerCompositor.cs index aa1f099cf6..cc6683e7fa 100644 --- a/src/Avalonia.Base/Rendering/Composition/Server/ServerCompositor.cs +++ b/src/Avalonia.Base/Rendering/Composition/Server/ServerCompositor.cs @@ -7,6 +7,7 @@ using Avalonia.Platform; using Avalonia.Rendering.Composition.Animations; using Avalonia.Rendering.Composition.Expressions; using Avalonia.Rendering.Composition.Transport; +using Avalonia.Threading; namespace Avalonia.Rendering.Composition.Server { @@ -20,7 +21,7 @@ namespace Avalonia.Rendering.Composition.Server { private readonly IRenderLoop _renderLoop; - private readonly Queue _batches = new Queue(); + private readonly Queue _batches = new Queue(); private readonly Queue _receivedJobQueue = new(); public long LastBatchId { get; private set; } public Stopwatch Clock { get; } = Stopwatch.StartNew(); @@ -32,6 +33,7 @@ namespace Avalonia.Rendering.Composition.Server internal BatchStreamMemoryPool BatchMemoryPool; private object _lock = new object(); private Thread? _safeThread; + private bool _uiThreadIsInsideRender; public PlatformRenderInterfaceContextManager RenderInterface { get; } internal static readonly object RenderThreadDisposeStartMarker = new(); internal static readonly object RenderThreadJobsStartMarker = new(); @@ -47,7 +49,7 @@ namespace Avalonia.Rendering.Composition.Server _renderLoop.Add(this); } - public void EnqueueBatch(Batch batch) + public void EnqueueBatch(CompositionBatch batch) { lock (_batches) _batches.Enqueue(batch); @@ -55,13 +57,13 @@ namespace Avalonia.Rendering.Composition.Server internal void UpdateServerTime() => ServerNow = Clock.Elapsed; - List _reusableToNotifyProcessedList = new(); - List _reusableToNotifyRenderedList = new(); + List _reusableToNotifyProcessedList = new(); + List _reusableToNotifyRenderedList = new(); void ApplyPendingBatches() { while (true) { - Batch batch; + CompositionBatch batch; lock (_batches) { if(_batches.Count == 0) @@ -154,20 +156,46 @@ namespace Avalonia.Rendering.Composition.Server } public void Render() + { + if (Dispatcher.UIThread.CheckAccess()) + { + if (_uiThreadIsInsideRender) + throw new InvalidOperationException("Reentrancy is not supported"); + _uiThreadIsInsideRender = true; + try + { + using (Dispatcher.UIThread.DisableProcessing()) + RenderReentrancySafe(); + } + finally + { + _uiThreadIsInsideRender = false; + } + } + else + RenderReentrancySafe(); + } + + private void RenderReentrancySafe() { lock (_lock) { try { - _safeThread = Thread.CurrentThread; - RenderCore(); + try + { + _safeThread = Thread.CurrentThread; + RenderCore(); + } + finally + { + NotifyBatchesRendered(); + } } finally { - NotifyBatchesRendered(); _safeThread = null; } - } } diff --git a/src/Avalonia.Base/Rendering/Composition/Server/SimpleServerObject.cs b/src/Avalonia.Base/Rendering/Composition/Server/SimpleServerObject.cs index daaa309bec..cb2d8dd84c 100644 --- a/src/Avalonia.Base/Rendering/Composition/Server/SimpleServerObject.cs +++ b/src/Avalonia.Base/Rendering/Composition/Server/SimpleServerObject.cs @@ -17,7 +17,7 @@ class SimpleServerObject } - public void DeserializeChanges(BatchStreamReader reader, Batch batch) + public void DeserializeChanges(BatchStreamReader reader, CompositionBatch batch) { DeserializeChangesCore(reader, batch.CommittedAt); ValuesInvalidated(); diff --git a/src/Avalonia.Base/Rendering/Composition/Transport/Batch.cs b/src/Avalonia.Base/Rendering/Composition/Transport/Batch.cs index c21f1c9227..480ed39363 100644 --- a/src/Avalonia.Base/Rendering/Composition/Transport/Batch.cs +++ b/src/Avalonia.Base/Rendering/Composition/Transport/Batch.cs @@ -9,16 +9,16 @@ namespace Avalonia.Rendering.Composition.Transport /// /// Represents a group of serialized changes from the UI thread to be atomically applied at the render thread /// - internal class Batch + public class CompositionBatch { private static long _nextSequenceId = 1; private static ConcurrentBag _pool = new(); private readonly TaskCompletionSource _acceptedTcs = new(); private readonly TaskCompletionSource _renderedTcs = new(); - public long SequenceId { get; } + internal long SequenceId { get; } - public Batch() + internal CompositionBatch() { SequenceId = Interlocked.Increment(ref _nextSequenceId); if (!_pool.TryTake(out var lst)) @@ -27,12 +27,34 @@ namespace Avalonia.Rendering.Composition.Transport } - public BatchStreamData Changes { get; private set; } - public TimeSpan CommittedAt { get; set; } + internal BatchStreamData Changes { get; private set; } + internal TimeSpan CommittedAt { get; set; } + + /// + /// Indicates that batch got deserialized on the render thread and will soon be rendered. + /// It's generally a good time to start producing the next one + /// + /// + /// To allow timing-sensitive code to receive the notification in time, the TaskCompletionSource + /// is configured to invoke continuations _synchronously_, so your `await` could happen from the render loop + /// if it happens to run on the UI thread. + /// It's recommended to use Dispatcher.AwaitOnPriority when consuming from the UI thread + /// public Task Processed => _acceptedTcs.Task; + + /// + /// Indicates that batch got rendered on the render thread. + /// It's generally a good time to start producing the next one + /// + /// + /// To allow timing-sensitive code to receive the notification in time, the TaskCompletionSource + /// is configured to invoke continuations _synchronously_, so your `await` could happen from the render loop + /// if it happens to run on the UI thread. + /// It's recommended to use Dispatcher.AwaitOnPriority when consuming from the UI thread + /// public Task Rendered => _renderedTcs.Task; - public void NotifyProcessed() + internal void NotifyProcessed() { _pool.Add(Changes); Changes = null!; @@ -40,6 +62,6 @@ namespace Avalonia.Rendering.Composition.Transport _acceptedTcs.TrySetResult(0); } - public void NotifyRendered() => _renderedTcs.TrySetResult(0); + internal void NotifyRendered() => _renderedTcs.TrySetResult(0); } } diff --git a/src/Avalonia.Base/Threading/Dispatcher.Invoke.cs b/src/Avalonia.Base/Threading/Dispatcher.Invoke.cs index add990bd57..904bd0b607 100644 --- a/src/Avalonia.Base/Threading/Dispatcher.Invoke.cs +++ b/src/Avalonia.Base/Threading/Dispatcher.Invoke.cs @@ -619,4 +619,16 @@ public partial class Dispatcher _ = action ?? throw new ArgumentNullException(nameof(action)); InvokeAsyncImpl(new SendOrPostCallbackDispatcherOperation(this, priority, action, arg, true), CancellationToken.None); } + + /// + /// Returns a task awaitable that would invoke continuation on specified dispatcher priority + /// + public DispatcherPriorityAwaitable AwaitWithPriority(Task task, DispatcherPriority priority) => + new(this, task, priority); + + /// + /// Returns a task awaitable that would invoke continuation on specified dispatcher priority + /// + public DispatcherPriorityAwaitable AwaitWithPriority(Task task, DispatcherPriority priority) => + new(this, task, priority); } diff --git a/src/Avalonia.Base/Threading/DispatcherPriorityAwaitable.cs b/src/Avalonia.Base/Threading/DispatcherPriorityAwaitable.cs new file mode 100644 index 0000000000..147a95dc1d --- /dev/null +++ b/src/Avalonia.Base/Threading/DispatcherPriorityAwaitable.cs @@ -0,0 +1,40 @@ +using System; +using System.Runtime.CompilerServices; +using System.Threading.Tasks; + +namespace Avalonia.Threading; + +public class DispatcherPriorityAwaitable : INotifyCompletion +{ + private readonly Dispatcher _dispatcher; + private protected readonly Task Task; + private readonly DispatcherPriority _priority; + + internal DispatcherPriorityAwaitable(Dispatcher dispatcher, Task task, DispatcherPriority priority) + { + _dispatcher = dispatcher; + Task = task; + _priority = priority; + } + + public void OnCompleted(Action continuation) => + Task.ContinueWith(_ => _dispatcher.Post(continuation, _priority)); + + public bool IsCompleted => Task.IsCompleted; + + public void GetResult() => Task.GetAwaiter().GetResult(); + + public DispatcherPriorityAwaitable GetAwaiter() => this; +} + +public class DispatcherPriorityAwaitable : DispatcherPriorityAwaitable +{ + internal DispatcherPriorityAwaitable(Dispatcher dispatcher, Task task, DispatcherPriority priority) : base( + dispatcher, task, priority) + { + } + + public new T GetResult() => ((Task)Task).GetAwaiter().GetResult(); + + public new DispatcherPriorityAwaitable GetAwaiter() => this; +} \ No newline at end of file