Skip to content
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

Cannot use stream-based I/O to ConPTY/Console and receive WINDOWS_BUFFER_SIZE_EVENT simultaneously #394

Closed
oising opened this issue Mar 25, 2019 · 15 comments
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Question For questions or discussion Product-Conhost For issues in the Console codebase Resolution-Answered Related to questions that have been answered

Comments

@oising
Copy link
Collaborator

oising commented Mar 25, 2019

Microsoft Windows [Version 10.0.18362.1]

To monitor console buffer resize events (and viewport resize events in more recent builds), ReadConsoleInput must be used to look for INPUT_BUFFER events with an event type of 4 ( WINDOW_BUFFER_SIZE_EVENT ).

My problem is that when I open a pseudoconsole session for a new cmd.exe process and I want to use bidirectional VT codes (stream based) for stdin/stdout for maximum cross platform portability. That is to say, I don't want to use the windows console APIs because they are windows specific. This seems to make it very difficult - without gag-inducing and complex task synchronization - to watch for resize events at the same time. When stdin/stdout are flushed, any INPUT_BUFFER records in ReadConsoleInput are too, including WINDOWS_BUFFER_SIZE_EVENT records.

VT allows capturing mouse movement - or at least it should in the future since ConPTY doesn't seem to forward mouse coords/actions yet, but there is no equivalent VT sequence for resizes of the viewport. Here's some demo/repro code:

My specific NuGet dependencies for the repro are:

  • Vanara.PInvoke.Kernel32 / 2.3.4
  • Pipelines.Sockets.Unofficial / 2.0.7
using System;
using System.IO;
using System.IO.Pipelines;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;

using Pipelines.Sockets.Unofficial;
using Vanara.PInvoke;

namespace resizetest
{
    internal class Program
    {
        private static async Task Main(string[] args)
        {
            // run for 30 seconds
            const int timeout = 30000;

            var source = new CancellationTokenSource(timeout); // 30 seconds
            var token = source.Token;
            var handle = Kernel32.GetStdHandle(Kernel32.StdHandleType.STD_INPUT_HANDLE);
            
            Kernel32.CONSOLE_MODE mode = default;

            if (!Kernel32.GetConsoleMode(handle, ref mode))
            {
                throw Marshal.GetExceptionForHR(Marshal.GetLastWin32Error());
            }
            
            mode |= Kernel32.CONSOLE_MODE.ENABLE_WINDOW_INPUT;
            mode |= Kernel32.CONSOLE_MODE.ENABLE_VIRTUAL_TERMINAL_INPUT;
            mode &= ~Kernel32.CONSOLE_MODE.ENABLE_MOUSE_INPUT;

            if (!Kernel32.SetConsoleMode(handle, mode))
            {
                throw Marshal.GetExceptionForHR(Marshal.GetLastWin32Error());
            }

            Task drainStdin = Task.Run(async () =>
            {
                // UNCOMMENT THESE LINES TO ENABLE CAPTURE OF RESIZE EVENTS
                //await Task.Delay(timeout, token);
                //return;

                Stream stdin = Console.OpenStandardInput();
                PipeReader reader = StreamConnection.GetReader(stdin);

                while (!token.IsCancellationRequested)
                {
                    var result = await reader.ReadAsync(token);
                    if (result.IsCanceled || result.IsCompleted)
                    {
                        break;
                    }

                    // do stuff...

                    Console.WriteLine("stdin: read {0} byte(s)", result.Buffer.Length);
                }
            }, token);

            Task readInputBuffer = Task.Run(async () =>
            {
                while (!token.IsCancellationRequested)
                {
                    await Task.Delay(10, token);

                    if (!Kernel32.GetNumberOfConsoleInputEvents(handle, out var count))
                    {
                        throw Marshal.GetExceptionForHR(Marshal.GetLastWin32Error());
                    }

                    if (count > 0)
                    {
                        var buffer = new Kernel32.INPUT_RECORD[count];

                        if (!Kernel32.ReadConsoleInput(handle, buffer, count, out var total)) {
                            throw Marshal.GetExceptionForHR(Marshal.GetLastWin32Error());
                        }

                        if (total > 0)
                        {
                            // TODO: de-bounce and only send last resize? 
                            for (int index = 0; index < total; index++)
                            {
                                var record = buffer[index];

                                Console.WriteLine("type: {0}", record.EventType);

                                if (record.EventType == Kernel32.EVENT_TYPE.WINDOW_BUFFER_SIZE_EVENT) // resize
                                {
                                    var size = record.Event.WindowBufferSizeEvent.dwSize;
                                    Console.WriteLine("buffer: resize to {{{0}, {1}}}", size.X, size.Y);
                                }
                            }
                        }
                    }
                }

            }, token);

            Console.WriteLine("Running");

            try
            {
                await Task.WhenAll(readInputBuffer, drainStdin);
            }
            catch (OperationCanceledException)
            {
                // timeout
            }

            Console.WriteLine("press any key...");
            Console.ReadKey(true);

        }
    }
}

TL;DR -- it's very hard, if not impossible, to watch for viewport resizing and also use a pure VT implementation (no console API).

Solution? I'd suggest draining the input buffer if streaming but leaving resize, menu, mouse (if enabled) events etc. This may be a memory leak (but probably fixed max, circular buffer etc) if the default behaviour changed, so I guess it should be opt-in.

@oising
Copy link
Collaborator Author

oising commented Mar 25, 2019

Just to put this in context, I'm writing a terminal multiplexer under coreclr (2.2). When I've bound conhost to a cmd/wsl/pwsh subprocess, I can't resize the virtual terminal in response to the user resizing conhost.

@oising oising changed the title Cannot use stream-based I/O to ConPTY/Console and ReadConsoleInput simultaneously Cannot use stream-based I/O to ConPTY/Console and receive WINDOWS_BUFFER_SIZE_EVENT simultaneously Mar 25, 2019
@miniksa miniksa added the Issue-Question For questions or discussion label Mar 27, 2019
@miniksa
Copy link
Member

miniksa commented Mar 27, 2019

@oising to be clear, are you trying to be a client application or a server application?

Your title is mixing both ConPTY and the usage of the classic console API, which implies to me that you're trying to be a client application and a server (console window host) to yourself at the same time.

Or is it that you're trying to be a client application but speaking 99% VT on WriteFile and only using the minimal amount of other Windows client-APIs as possible?

EDIT: I'm dumb. Let me re-read this and try again.

@miniksa
Copy link
Member

miniksa commented Mar 27, 2019

OK so you've got this:

(A) Conhost in standard window visible mode <----> (B) Multiplexer application <------> (C) Conhost in ConPty mode <-----> (D) Client application

A is the server on top displaying stuff.
B is the client of the A server and is simultaneously holding the server end of the C ConPty.
The client end of the C ConPty is going to the D client application.

D is probably speaking classic Win32 to the C conhost in ConPTY mode.
The C conhost in ConPTY is then presumably speaking VT out to B (and vice versa).
The B multiplexer on startup right after attaching to A tells the A handles that it wants the virtual terminal output and input modes so it can get/give information in those formats over the STDIN and STDOUT pipes to make things easy with the information it's receiving from the other side from C.

And I think your problem is that between A and B, it's difficult or nigh impossible to know when the A window has resized and have an event come into B in a format that is easy to consume.

Sound right?

@oising
Copy link
Collaborator Author

oising commented Mar 27, 2019

@miniksa Right. Between A (physical, visible conhost window) and B (multiplexer watching conpty C running D, and relaying stdin/stdout between D and A.)

If I resize the physical window (A), it seems impossible to capture that informational event while streaming stdin/stdout to and from (A) and (D).

D is speaking classic win32 yes, but since it's conpty, it doesn't really matter.

@oising
Copy link
Collaborator Author

oising commented Mar 27, 2019

It seems that the moment you start reading from Console.OpenInputStream() then ReadConsoleInput stops receiving any data. Now obviously this makes sense at first glance, since you're gonna use one or the other but unfortunately resize events are only consumable from the latter.

(update: I invited you to my private repo hosting the full code if you're interested in taking a look for more context)

@miniksa
Copy link
Member

miniksa commented Mar 27, 2019

I think that the .NET abstractions here cause inadvertant flushes. If you're trying to do something advanced like this, I would strongly discourage using System.Console and recommend you just P/Invoke the Win32 APIs instead.

I'll have to give your private repo and the rest of this a more thorough scrub tomorrow morning. I have a few other things to get through today. I might also need @zadjii-msft to get back to work to confer with him on the state of resize as he's been in that space more recently than I have (he's just on vacation this week, due back on Monday.)

@oising
Copy link
Collaborator Author

oising commented Mar 28, 2019

I dug into FX already and there was nothing there that stood out initially. Internally the console PAL layer uses ReadConsole or ReadFile, depending on whether or not the handle is redirected. Buried halfway down the ReadConsole docs, there's the following ominous statement:

If the input buffer contains input events other than keyboard events (such as mouse events or window-resizing events), they are discarded. Those events can only be read by using the ReadConsoleInput function.

Trying to force ReadFile and figure out the magic sequences was shutdown by the notes for ENABLE_WINDOW_INPUT:

User interactions that change the size of the console screen buffer are reported in the console's input buffer. Information about these events can be read from the input buffer by applications using the ReadConsoleInput function, but not by those using ReadFile or ReadConsole.

I suppose this shouldn't be a surprise since ReadConsole and ReadConsoleInput drain the same buffer. My vision of having a nice, clean symmetrical usage of pipelines for stdin/stdout has now been crushed.

Blergh.

@miniksa
Copy link
Member

miniksa commented Mar 28, 2019

Yes, sorry, uhhh... When the calls are serviced, it's the same queue underlying both APIs inside the conhost.exe process. So if you try to ReadFile a thing (or ReadConsole) and it hits the queue and a non KEY_EVENT is sitting there, it's discarded and moves onto the next event.

Also, I have a massive pile of e-mail in my inbox this morning so I'm not remotely closer to looking at your private repro that you shared with me. Please bear with me. I'll try to get to it.

@oising
Copy link
Collaborator Author

oising commented Mar 28, 2019

@miniksa Don't stress man over the private repo. It's something you might enjoy reading through to get an idea how someone is trying to do something more advanced than a sample app. In your own time!

@oising
Copy link
Collaborator Author

oising commented Mar 28, 2019

Okay, so it looks like I've no choice but to wrap ReadConsoleInput as a Stream, just as the console wraps ReadConsole and come up with some way to side channel the other events. Not trivial, but not impossible either. You guys can either close this, or keep it open for tracking if you see this as an API improvement possibility.

@miniksa
Copy link
Member

miniksa commented Mar 29, 2019

This is one of the classes of issue I want to solve when we get time to do something about #281. Might leave it open.

@oising
Copy link
Collaborator Author

oising commented Apr 11, 2019

So I managed to hack together a prototype WindowsConsoleInputStream class that accepts a BlockingCollection<INPUT_RECORD> for side-channel event dispatching (MENU_EVENT, MOUSE_EVENT, WINDOWS_BUFFER_SIZE_EVENT etc) while preserving byte array stream semantics for KEY_EVENT. This lets us wrap a PipeReader around it for nice and simple async IO on stdin/stdou with ConPTY, while watching for viewport changes asynchronously too. So, it's doable!

@miniksa
Copy link
Member

miniksa commented Apr 11, 2019

Excellent to hear. If you have a useful reusable component that helps make this easier, I'd be happy to accept a pull into this repo in the tools directory so we can have it for others to use as well.

@oising
Copy link
Collaborator Author

oising commented Apr 21, 2019

@miniksa Done - #414

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label May 17, 2019
@miniksa miniksa added Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Product-Conhost For issues in the Console codebase Resolution-Answered Related to questions that have been answered and removed Mass-Chaos labels May 17, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 18, 2019
@miniksa
Copy link
Member

miniksa commented May 18, 2019

I think this is answered, done, and relatively over with for now. Any further calculations on getting the event out will have to do with that one that I said "I need to make this a deliverable for size events".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Question For questions or discussion Product-Conhost For issues in the Console codebase Resolution-Answered Related to questions that have been answered
Projects
None yet
Development

No branches or pull requests

2 participants