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

Epic: Make input and output mode process state instead of global state #4954

Open
DHowett-MSFT opened this issue Mar 17, 2020 · 2 comments
Open
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Scenario Product-Conhost For issues in the Console codebase
Milestone

Comments

@DHowett-MSFT
Copy link
Contributor

DHowett-MSFT commented Mar 17, 2020

Notes:

The input and output modes are global state, and while we're thinking about what it would take to change that we're definitely concerned about backwards compatibility.
When an application exits and leaves the console in a state different from the one it has when it launched (which can happen for a number of reasons: it wasn't authored correctly, it crashes, it returns control of the console so the shell before it's ready (#4921)), we have little choice but to get into a weird state.
You see this with most terminals on the market, unfortunately. If vim is running in mouse mode, and is abruptly terminated, suddenly moving your mouse over your terminal generates an absolute heap of garbage input. It's impossible to solve in that case, but given that we own the console subsystem in its entirety we can put together a better story.

We should investigate our options here: can we make the input or output mode process state?

If we make the input mode process-local, we need to move to an input model that miniksa and I have discussed: input records are translated (mouse -> vt, input -> vt, clipboard -> key, or clipboard -> vt) for the reader instead of upon insertion into the queue. That's a huge chunk of work.

Making the output mode process-local may have many fewer caveats. We already have some process-local output state like UTF-8 partials.

This'll solve the overarching issue in #4921 and #4949.

@DHowett-MSFT DHowett-MSFT added Product-Conhost For issues in the Console codebase Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Scenario labels Mar 17, 2020
@DHowett-MSFT DHowett-MSFT added this to the Console Backlog milestone Mar 17, 2020
@DHowett-MSFT DHowett-MSFT added this to Spec Needed ❓ in Specification Tracker via automation Mar 17, 2020
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Mar 17, 2020
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Mar 17, 2020
@DHowett
Copy link
Member

DHowett commented Jul 30, 2020

I attempted to dissect our input queue during Hackathon. Here's my notes


inside the buffer

Things that happen during WRITE

  • If VT_INPUT_MODE, use TerminalInput to convert key events into key events
  • if we're writing one event at a time, coalesce key and/or mouse events
    • COMPAT must only coalesce when writing one-by-one to maintain v1 contract
    • temporarily prepend this event to the queue
    • if we didn't coalesce, go ahead and unprepend
  • (sometimes) wake up waiting readers

Things that happen during READ

  • Decoalescing
    • Decoalescing changes the live state of the input buffer, under lock
    • When we are decoalescing key events during streaming read, we actually decrement the repeat
      count in the live event
  • actually delete events from the queue
  • count "virtual read" if glyph is full width (dbcs records count for 2 virtual reads)
  • if we were peeking, manually restore every event that we just messed up
    • we recreate the input event by going from IInputEvent->INPUT_RECORD->IInputEvent (?!)
  • reset the wait event if the buffer is empty

During FLUSH

  • Delete _storage ONLY.
  • DOES NOT clear enqueued partials (!)

outside the buffer

  • GetChar
    • This is the only known user of InputBuffer::Read(single event)
  • (Read|Peek)ConsoleInput[WA] (_DoGetConsoleInput w/ args)
    1. Are there partials? Pop them out of the cache
    2. Begin a read
    3. If we're told to wait, start up a DirectReadData
    4. convert events (if A)
    5. store partials if they won't fit
  • ReadConsole[AW] (ReadConsole + ReadFile both go here)
    • ReadFile comes in with a zeroed read struct, so Unicode=FALSE
    • These methods only read as strings...
    • If there is input pending, read it
      • seems like there's a fork over whether we're multiline
      • if multiline, stop after the first LINEFEED (0x0a)
      • it is possible to have more pending input
      • translate to ANSI
    • line input mode?
      • COOKED_READ -- DO NOT TOUCH THIS
    • read by character
      1. if there's a partial, emit it
        • out of space? stop, we're done
      2. read one char
        see GetChar above
      3. If we're forced to wait, fork off a RAW_READ_DATA
      4. read a bunch more chars
      5. translate unicode to OEM

ENQUEUED EVENTS

  • DirectReadData
    • Direct copy of the code in _DoGetConsoleInput from 2+
  • RAW_READ_DATA
    • Direct copy of the code in "read by character" from 4+

@DHowett
Copy link
Member

DHowett commented Jul 30, 2020

The concerns for handling partials are spread across three classes, some of which are already process-local state. If you do a LINE_INPUT and receive multiple lines, there's a chance that partial lines will be saved against your process data. If you do a raw event input and get partials, they're saved globally on the queue. Neither of these are cleared by a FLUSH.

Rearchitecting our input records to do translation at READ time instead of WRITE time will be difficult, but will ensure more consistent results (one process can't read half of a VT sequence, the clipboard would be a single event so that two processes couldn't both read half the clipboard data, etc.) and when an event translates to multiple outputs that don't fit they'll be enqueued per-handle.

@zadjii-msft zadjii-msft modified the milestones: Console Backlog, Backlog Jan 4, 2022
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-Scenario Product-Conhost For issues in the Console codebase
Projects
Specification Tracker
  
Spec Needed ❓
Development

No branches or pull requests

3 participants