Skip to content

Commit

Permalink
Merge pull request #3567 from SamBent/HostVisualThreading
Browse files Browse the repository at this point in the history 
HostVisual threading
  • Loading branch information
@SamBent
SamBent authored Oct 7, 2020
2 parents 45f3f33 + e628574 commit cf8bd1f
Show file tree
Hide file tree
Showing 2 changed files with 212 additions and 24 deletions.
219 changes: 195 additions & 24 deletions src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/HostVisual.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,16 @@ internal override void FreeContent(DUCE.Channel channel)

using (CompositionEngineLock.Acquire())
{
DisconnectHostedVisual(
channel,
/* removeChannelFromCollection */ true);
// if there's a pending disconnect, do it now preemptively;
// otherwise do the disconnect the normal way.
// This ensures we do the disconnect before calling base,
// as required.
if (!DoPendingDisconnect(channel))
{
DisconnectHostedVisual(
channel,
/* removeChannelFromCollection */ true);
}
}

base.FreeContent(channel);
Expand Down Expand Up @@ -252,7 +259,7 @@ private void EnsureHostedVisualConnected(DUCE.Channel channel)
//
if (!(channel.IsSynchronous)
&& _target != null
&& !_connectedChannels.Contains(channel))
&& !_connectedChannels.ContainsKey(channel))
{
Debug.Assert(IsOnChannel(channel));

Expand Down Expand Up @@ -323,7 +330,15 @@ private void EnsureHostedVisualConnected(DUCE.Channel channel)
channel);
}

_connectedChannels.Add(channel);
// remember what channel we connected to, and which thread
// did the connection, so that we can disconnect on the
// same thread. Earlier comments imply this is the HostVisual's
// dispatcher thread, which we assert here. Even if it's not,
// the code downstream should work, or at least not crash
// (even if channelDispatcher is set to null).
Dispatcher channelDispatcher = Dispatcher.FromThread(Thread.CurrentThread);
Debug.Assert(channelDispatcher == this.Dispatcher, "HostVisual connecting on a second thread");
_connectedChannels.Add(channel, channelDispatcher);

//
// Indicate that that content composition root has been
Expand Down Expand Up @@ -364,10 +379,11 @@ private void EnsureHostedVisualConnected(DUCE.Channel channel)
/// </summary>
private void DisconnectHostedVisualOnAllChannels()
{
foreach (DUCE.Channel channel in _connectedChannels)
IDictionaryEnumerator ide = _connectedChannels.GetEnumerator() as IDictionaryEnumerator;
while (ide.MoveNext())
{
DisconnectHostedVisual(
channel,
(DUCE.Channel)ide.Key,
/* removeChannelFromCollection */ false);
}

Expand All @@ -382,23 +398,41 @@ private void DisconnectHostedVisual(
DUCE.Channel channel,
bool removeChannelFromCollection)
{
if (_target != null && _connectedChannels.Contains(channel))
Dispatcher channelDispatcher;
if (_target != null && _connectedChannels.TryGetValue(channel, out channelDispatcher))
{
DUCE.CompositionNode.RemoveChild(
_proxy.GetHandle(channel),
_target._contentRoot.GetHandle(channel),
channel
);

//
// Release the targets handle. If we had duplicated the handle,
// then this removes the duplicated handle, otherwise just decrease
// the ref count for VisualTarget.
//

_target._contentRoot.ReleaseOnChannel(channel);

SetFlags(channel, false, VisualProxyFlags.IsContentNodeConnected);
// Adding commands to a channel is not thread-safe,
// we must do the actual work on the same dispatcher thread
// where the connection happened.
if (channelDispatcher != null && channelDispatcher.CheckAccess())
{
Disconnect(channel,
channelDispatcher,
_proxy.GetHandle(channel),
_target._contentRoot.GetHandle(channel),
_target._contentRoot);
}
else
{
// marshal to the right thread
if (channelDispatcher != null)
{
DispatcherOperation op = channelDispatcher.BeginInvoke(
DispatcherPriority.Normal,
new DispatcherOperationCallback(DoDisconnectHostedVisual),
channel);

_disconnectData = new DisconnectData(
op: op,
channel: channel,
dispatcher: channelDispatcher,
hostVisual: this,
hostHandle: _proxy.GetHandle(channel),
targetHandle: _target._contentRoot.GetHandle(channel),
contentRoot: _target._contentRoot,
next: _disconnectData);
}
}

if (removeChannelFromCollection)
{
Expand All @@ -407,6 +441,101 @@ private void DisconnectHostedVisual(
}
}

/// <summary>
/// Callback to disconnect on the right thread
/// </summary>
private object DoDisconnectHostedVisual(object arg)
{
using (CompositionEngineLock.Acquire())
{
DoPendingDisconnect((DUCE.Channel)arg);
}

return null;
}

/// <summary>
/// Perform a pending disconnect for the given channel.
/// This method should be called under the CompositionEngineLock,
/// on the thread that owns the channel. It can be called either
/// from the dispatcher callback DoDisconnectHostedVisual or
/// from FreeContent, whichever happens to occur first.
/// </summary>
/// <returns>
/// True if a matching request was found and processed. False if not.
/// </returns>
private bool DoPendingDisconnect(DUCE.Channel channel)
{
DisconnectData disconnectData = _disconnectData;
DisconnectData previous = null;

// search the list for an entry matching the given channel
while (disconnectData != null && (disconnectData.HostVisual != this || disconnectData.Channel != channel))
{
previous = disconnectData;
disconnectData = disconnectData.Next;
}

// if no match found, do nothing
if (disconnectData == null)
{
return false;
}

// remove the matching entry from the list
if (previous == null)
{
_disconnectData = disconnectData.Next;
}
else
{
previous.Next = disconnectData.Next;
}

// cancel the dispatcher callback, (if we're already in it,
// this call is a no-op)
disconnectData.DispatcherOperation.Abort();

// do the actual disconnect
Disconnect(disconnectData.Channel,
disconnectData.ChannelDispatcher,
disconnectData.HostHandle,
disconnectData.TargetHandle,
disconnectData.ContentRoot);

return true;
}

/// <summary>
/// Do the actual work to disconnect the VisualTarget.
/// This is called (on the channel's thread) either from
/// DisconnectHostedVisual or from DoPendingDisconnect,
/// depending on which thread the request arrived on.
/// </summary>
private void Disconnect(DUCE.Channel channel,
Dispatcher channelDispatcher,
DUCE.ResourceHandle hostHandle,
DUCE.ResourceHandle targetHandle,
DUCE.MultiChannelResource contentRoot)
{
channelDispatcher.VerifyAccess();

DUCE.CompositionNode.RemoveChild(
hostHandle,
targetHandle,
channel
);

//
// Release the targets handle. If we had duplicated the handle,
// then this removes the duplicated handle, otherwise just decrease
// the ref count for VisualTarget.
//

contentRoot.ReleaseOnChannel(channel);

SetFlags(channel, false, VisualProxyFlags.IsContentNodeConnected);
}

/// <summary>
/// Invalidate this visual.
Expand Down Expand Up @@ -443,7 +572,49 @@ private void Invalidate()
/// <remarks>
/// This field is free-threaded and should be accessed from under a lock.
/// </remarks>
private List<DUCE.Channel> _connectedChannels = new List<DUCE.Channel>();
private Dictionary<DUCE.Channel, Dispatcher> _connectedChannels = new Dictionary<DUCE.Channel, Dispatcher>();

/// <summary>
/// Data needed to disconnect the visual target.
/// </summary>
/// <remarks>
/// This field is free-threaded and should be accessed from under a lock.
/// It's the head of a singly-linked list of pending disconnect requests,
/// each identified by the channel and HostVisual. In practice, the list
/// is either empty or has only one entry.
/// </remarks>
private static DisconnectData _disconnectData;

private class DisconnectData
{
public DispatcherOperation DispatcherOperation { get; private set; }
public DUCE.Channel Channel { get; private set; }
public Dispatcher ChannelDispatcher { get; private set; }
public HostVisual HostVisual { get; private set; }
public DUCE.ResourceHandle HostHandle { get; private set; }
public DUCE.ResourceHandle TargetHandle { get; private set; }
public DUCE.MultiChannelResource ContentRoot { get; private set; }
public DisconnectData Next { get; set; }

public DisconnectData(DispatcherOperation op,
DUCE.Channel channel,
Dispatcher dispatcher,
HostVisual hostVisual,
DUCE.ResourceHandle hostHandle,
DUCE.ResourceHandle targetHandle,
DUCE.MultiChannelResource contentRoot,
DisconnectData next)
{
DispatcherOperation = op;
Channel = channel;
ChannelDispatcher = dispatcher;
HostVisual = hostVisual;
HostHandle = hostHandle;
TargetHandle = targetHandle;
ContentRoot = contentRoot;
Next = next;
}
}

#endregion Private Fields
}
Expand Down
17 changes: 17 additions & 0 deletions src/Microsoft.DotNet.Wpf/src/WpfGfx/include/exports.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,15 @@ internal struct ChannelSet
/// channel.
/// </summary>
internal sealed partial class Channel
#if ENFORCE_CHANNEL_THREAD_ACCESS
: System.Windows.Threading.DispatcherObject
// "Producer" operations - adding commands et al. - should only be done
// on the thread that created the channel. These operations are on the
// hot path, so we don't add the cost of enforcement. To detect
// violations (which can lead to render-thread failures that
// are very difficult to diagnose), build
// PresentationCore with ENFORCE_CHANNEL_THREAD_ACCESS defined.
#endif
{
/// <summary>
/// Primary channel.
Expand Down Expand Up @@ -768,6 +777,10 @@ unsafe internal void SendCommand(
int cSize,
bool sendInSeparateBatch)
{
#if ENFORCE_CHANNEL_THREAD_ACCESS
VerifyAccess();
#endif

checked
{
Invariant.Assert(pCommandData != (byte*)0 && cSize > 0);
Expand Down Expand Up @@ -808,6 +821,10 @@ unsafe internal void BeginCommand(
int cbSize,
int cbExtra)
{
#if ENFORCE_CHANNEL_THREAD_ACCESS
VerifyAccess();
#endif

checked
{
Invariant.Assert(cbSize > 0);
Expand Down

0 comments on commit cf8bd1f

Please sign in to comment.