Skip to content

WIP: Wrap ReadLine with lock and fallback to legacy when re-entering #1729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

andyleejordan
Copy link
Member

Just me trying things. So, weird thing, but in the repro for PowerShell/vscode-powershell#3751, while we're in the debugger, things can't print. Like if you run Get-Process at the debug prompt it shows nothing, and if you're debugging the pwsh process you see a million exceptions. This gets resolved when I set the host explicitly in the new PSInvocationSettings. When we either don't create the linked cancellation token source, or when we move its scope outside the function its currently created in, the repro issue goes away. The monitor logic I guess is useless 🥲

Play around with it @SeeminglyScience?

{
SetCanceled();
return;
}

try
{
TResult result = Run(cancellationSource.Token);
TResult result = Run(executorCancellationToken);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright so think of a linked cancellation token source kinda like this:

static void MyMethod(CancellationToken callersToken)
{
    CancellationTokenSource myToken = new(millisecondsDelay: 5000);

    // Real method name is "Register", but bare with me
    callersToken.OnCancel(() => myToken.Cancel());
    OtherMethod(myToken.Token);
}

That's all linking does. The issue with the WIP here is instead of passing myToken.Token, you're passing callersToken. So if we call myToken.Cancel(), that's not going to do anything. That's also why it looks like it works: it's never cancelled.

I haven't played with it yet, but I would guess we can no longer cancel PSRL like this (e.g. F8/editor commands/continue). If we can still cancel PSRL, we should probably look at references for _taskRequesterCancellationToken to see if we ever even cancel it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops I meant to still send cancellationSource.Token so I clearly didn't test what I thought I was testing.

@andyleejordan andyleejordan changed the title WIP WIP: Wrap ReadLine with lock and fallback to legacy when re-entering Jun 3, 2022
@andyleejordan andyleejordan deleted the andschwa/experiment branch February 12, 2024 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants