Skip to content

Prepare to work with newer PSES versions #156

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

Merged
merged 2 commits into from
Oct 4, 2023
Merged

Conversation

ForNeVeR
Copy link
Collaborator

@ForNeVeR ForNeVeR commented Oct 4, 2023

#104 uncovered several issues in how we work with PSES.

  1. First of all, our delayed process output reading strategy seemingly blocked the PSES process from starting.

    Before this PR, our PSES startup routine was the following:

    1. Start the process, passing it a path to a session file (assuming it will write its session info to the file).
    2. Wait for the session file to appear.
    3. Start reading the process stdout and stderr, piping it into our own logs.

    The current bundled PSES version (v1.10.1, I think?) is okay with this: it will happily write the session file and then start writing some bits of diagnostic output to its stdout.

    But v3.12.0 is different. It really insists on writing the output first, and then opening the file.

    But we do not read the stdout pipe before we see the file on disk! And thus its Console.Write (or whatever they are using) gets blocked on us. And we are waiting for them. A classical deadlock, albeit distributed between the two processes.

    I fixed that by starting processing the process output pipes right after its startup.

  2. And then, a second issue appeared: new PSES opens its LSP pipe with exclusive write access (seemingly it wasn't the case before). And we had a small snippet that tried to open the diagnostic pipe with a bit too wide permission (it requested write access to the pipe that's only meant to be written by the PSES process, not ours). Which caused a file access violation.

Both of the issues are fixed, but PSES still doesn't work: I think it uses a protocol newer than supported by our LSP library version.

So, I am going to close #104 by this PR, but process the remaining work on updating PSES in #51, as planned before.

@ForNeVeR ForNeVeR self-assigned this Oct 4, 2023
@ForNeVeR ForNeVeR merged commit 7a1ff1e into master Oct 4, 2023
@ForNeVeR ForNeVeR deleted the bugfix/104.diagnostics branch October 4, 2023 20:55
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.

Completion not working with custom PowerShell Editor Services
1 participant