Skip to content

Commit

Permalink
Attempt to fix issue with native apps and input
Browse files Browse the repository at this point in the history
On Unix like platforms some native applications do not work properly if
our event subscriber is active. I suspect this is due to PSReadLine
querying cursor position prior to checking for events. I believe the
cursor position response emitted is being read as input.

I've attempted to fix this by hooking into PSHost.NotifyBeginApplication
to temporarly remove the event subscriber, and
PSHost.NotifyEndApplication to recreate it afterwards.
  • Loading branch information
SeeminglyScience committed Aug 19, 2018
1 parent b51cc75 commit 1682410
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ public override void NotifyBeginApplication()
{
Logger.Write(LogLevel.Verbose, "NotifyBeginApplication() called.");
this.hostUserInterface.IsNativeApplicationRunning = true;
this.powerShellContext.NotifyBeginApplication();
}

/// <summary>
Expand All @@ -280,6 +281,7 @@ public override void NotifyEndApplication()
{
Logger.Write(LogLevel.Verbose, "NotifyEndApplication() called.");
this.hostUserInterface.IsNativeApplicationRunning = false;
this.powerShellContext.NotifyEndApplication();
}

/// <summary>
Expand Down
141 changes: 112 additions & 29 deletions src/PowerShellEditorServices/Session/InvocationEventQueue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Microsoft.PowerShell.EditorServices.Session
/// <summary>
/// Provides the ability to take over the current pipeline in a runspace.
/// </summary>
internal class InvocationEventQueue
internal class InvocationEventQueue : IDisposable
{
private const string ShouldProcessInExecutionThreadPropertyName = "ShouldProcessInExecutionThread";

Expand All @@ -35,24 +35,125 @@ internal class InvocationEventQueue

private readonly PowerShellContext _powerShellContext;

private readonly ILogger _logger;

private bool _isDisposed;

private InvocationRequest _invocationRequest;

private SemaphoreSlim _lock = AsyncUtils.CreateSimpleLockingSemaphore();
private PSEventSubscriber _onIdleSubscriber;

private SemaphoreSlim _pipelineRequestHandle = AsyncUtils.CreateSimpleLockingSemaphore();

private SemaphoreSlim _subscriberHandle = AsyncUtils.CreateSimpleLockingSemaphore();

private InvocationEventQueue(PowerShellContext powerShellContext, PromptNest promptNest)
private InvocationEventQueue(
PowerShellContext powerShellContext,
PromptNest promptNest,
ILogger logger)
{
_promptNest = promptNest;
_powerShellContext = powerShellContext;
_runspace = powerShellContext.CurrentRunspace.Runspace;
_logger = logger;
}

public void Dispose() => Dispose(true);

protected virtual void Dispose(bool disposing)
{
if (!_isDisposed)
{
return;
}

if (disposing)
{
RemoveInvocationSubscriber();
}

_isDisposed = true;
}

internal static InvocationEventQueue Create(PowerShellContext powerShellContext, PromptNest promptNest)
internal static InvocationEventQueue Create(
PowerShellContext powerShellContext,
PromptNest promptNest,
ILogger logger)
{
var eventQueue = new InvocationEventQueue(powerShellContext, promptNest);
var eventQueue = new InvocationEventQueue(powerShellContext, promptNest, logger);
eventQueue.CreateInvocationSubscriber();
return eventQueue;
}

/// <summary>
/// Creates a <see cref="PSEventSubscriber" /> subscribed the engine event
/// <see cref="PSEngineEvent.OnIdle" /> that handles requests for pipeline thread access.
/// </summary>
/// <returns>
/// The newly created <see cref="PSEventSubscriber" /> or an existing subscriber if
/// creation already occurred.
/// </returns>
internal PSEventSubscriber CreateInvocationSubscriber()
{
_subscriberHandle.Wait();
try
{
if (_onIdleSubscriber != null)
{
_logger.Write(
LogLevel.Error,
"An attempt to create the ReadLine OnIdle subscriber was made when one already exists.");
return _onIdleSubscriber;
}

_onIdleSubscriber = _runspace.Events.SubscribeEvent(
source: null,
eventName: PSEngineEvent.OnIdle,
sourceIdentifier: PSEngineEvent.OnIdle,
data: null,
handlerDelegate: OnPowerShellIdle,
supportEvent: true,
forwardEvent: false);

SetSubscriberExecutionThreadWithReflection(_onIdleSubscriber);

_onIdleSubscriber.Unsubscribed += OnInvokerUnsubscribed;

return _onIdleSubscriber;
}
finally
{
_subscriberHandle.Release();
}
}

/// <summary>
/// Unsubscribes the existing <see cref="PSEventSubscriber" /> handling pipeline thread
/// access requests.
/// </summary>
internal void RemoveInvocationSubscriber()
{
_subscriberHandle.Wait();
try
{
if (_onIdleSubscriber == null)
{
_logger.Write(
LogLevel.Error,
"An attempt to remove the ReadLine OnIdle subscriber was made before it was created.");
return;
}

_onIdleSubscriber.Unsubscribed -= OnInvokerUnsubscribed;
_runspace.Events.UnsubscribeEvent(_onIdleSubscriber);
_onIdleSubscriber = null;
}
finally
{
_subscriberHandle.Release();
}
}

/// <summary>
/// Executes a command on the main pipeline thread through
/// eventing. A <see cref="PSEngineEvent.OnIdle" /> event subscriber will
Expand Down Expand Up @@ -136,7 +237,7 @@ internal async Task InvokeOnPipelineThread(Action<PowerShell> invocationAction)
private async Task WaitForExistingRequestAsync()
{
InvocationRequest existingRequest;
await _lock.WaitAsync();
await _pipelineRequestHandle.WaitAsync();
try
{
existingRequest = _invocationRequest;
Expand All @@ -147,7 +248,7 @@ private async Task WaitForExistingRequestAsync()
}
finally
{
_lock.Release();
_pipelineRequestHandle.Release();
}

await existingRequest.Task;
Expand All @@ -156,22 +257,22 @@ private async Task WaitForExistingRequestAsync()
private async Task SetInvocationRequestAsync(InvocationRequest request)
{
await WaitForExistingRequestAsync();
await _lock.WaitAsync();
await _pipelineRequestHandle.WaitAsync();
try
{
_invocationRequest = request;
}
finally
{
_lock.Release();
_pipelineRequestHandle.Release();
}

_powerShellContext.ForcePSEventHandling();
}

private void OnPowerShellIdle(object sender, EventArgs e)
{
if (!_lock.Wait(0))
if (!_pipelineRequestHandle.Wait(0))
{
return;
}
Expand All @@ -188,7 +289,7 @@ private void OnPowerShellIdle(object sender, EventArgs e)
}
finally
{
_lock.Release();
_pipelineRequestHandle.Release();
}

_promptNest.PushPromptContext();
Expand All @@ -202,24 +303,6 @@ private void OnPowerShellIdle(object sender, EventArgs e)
}
}

private PSEventSubscriber CreateInvocationSubscriber()
{
PSEventSubscriber subscriber = _runspace.Events.SubscribeEvent(
source: null,
eventName: PSEngineEvent.OnIdle,
sourceIdentifier: PSEngineEvent.OnIdle,
data: null,
handlerDelegate: OnPowerShellIdle,
supportEvent: true,
forwardEvent: false);

SetSubscriberExecutionThreadWithReflection(subscriber);

subscriber.Unsubscribed += OnInvokerUnsubscribed;

return subscriber;
}

private void OnInvokerUnsubscribed(object sender, PSEventUnsubscribedEventArgs e)
{
CreateInvocationSubscriber();
Expand Down
25 changes: 25 additions & 0 deletions src/PowerShellEditorServices/Session/PowerShellContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,30 @@ await this.ExecuteCommand<object>(
addToHistory: true);
}

/// <summary>
/// Called by the active <see cref="EditorServicesPSHost" /> to prepare for a native
/// application execution.
/// </summary>
internal void NotifyBeginApplication()
{
// The OnIdle subscriber causes PSReadLine to query cursor position periodically. On
// Unix based platforms this can cause native applications to read the cursor position
// response query emitted to STDIN as input.
this.InvocationEventQueue.RemoveInvocationSubscriber();
}

/// <summary>
/// Called by the active <see cref="EditorServicesPSHost" /> to cleanup after a native
/// application execution.
/// </summary>
internal void NotifyEndApplication()
{
// The OnIdle subscriber causes PSReadLine to query cursor position periodically. On
// Unix based platforms this can cause native applications to read the cursor position
// response query emitted to STDIN as input.
this.InvocationEventQueue.CreateInvocationSubscriber();
}

/// <summary>
/// Forces the <see cref="PromptContext" /> to trigger PowerShell event handling,
/// reliquishing control of the pipeline thread during event processing.
Expand Down Expand Up @@ -1246,6 +1270,7 @@ private void ResumeDebugger(DebuggerResumeAction resumeAction, bool shouldWaitFo
public void Dispose()
{
this.PromptNest.Dispose();
this.InvocationEventQueue.Dispose();
this.SessionState = PowerShellContextState.Disposed;

// Clean up the active runspace
Expand Down

0 comments on commit 1682410

Please sign in to comment.