-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Crash when defterm handoff'ing a bat file? #15245
Comments
Oh and it worked fine on 1.16.10262.0 |
............ aaaaaand it works fine in a dev build. I'm guessing this is just more of my machine's "defterm is messed up yo". That or 1041 had bugs (not unlikely) |
HMMMMMM I GOT IT TO HAPPEN AGAIN
Wow that all looks super fucked. |
AND there are crashes for this that aren't just me! |
Oh dear In the PtySignal thread:
The signal thread is waiting for a chance to resize the conpty to (in this case) (0x52,0x19). BUT IT KEEPS ON GOING ANYWAYS. It's printed 26 lines... to.. the... textbuffer? I'd reckon this might even be the root cause of #14512. There's a miscommunication between the Terminal and conpty on how big it should be. |
Interesting. Looking through the dump, the text buffer of
It's the one in the actual adapter call that looks messed up. I don't think anyone can go through and change the screen buffer while the ConsoleIO thread is processing that string, but maybe I'm just bad at windbg: |
Thoughts while trying to fall asleep:
|
Let's assume my windbg skills around pointers/refs is just bad. REGARDLESS, in this call to
|
NOW here's a question. Why was the original text buffer 9001 rows tall? Did something happen to not immediately replace the text buffer? If
Okay, so that first screen info is created after we setup handoff. Maybe there's something in the way it's created that ignores the VtIo setup we do for defterm. Maybe we never noticed, cause terminal's usually really good about immediately requesting a resize, but in this case, the CLI app really wants to get going, before Terminal's even had it's coffee |
we're really gonna gloss over that huh?
|
This is a theory. I want to audit the uses of `CONSOLE_INFORMATION::IsInVtIoMode` before I commit to this. But I think this: * fixes #15245 * fixes #14512 Because basically, we'd create the first screen buffer with 9001 rows, because it would be created _before_ VtIo would be in a state to say "yes, we're a conpty"
We need to act like a ConPTY just a little earlier in startup. My relevant notes start here: #15245 (comment). Basically, we'd create the first screen buffer with 9001 rows, because it would be created _before_ VtIo would be in a state to say "yes, we're a conpty". Then, if a CLI app emits an entire screenful of text _before_ the terminal has a chance to resize the conpty, then the conpty will explode during `_DoLineFeed`. That method is absolutely not expecting the buffer to get resized (and the old text buffer deallocated). Instead, this will treat the console as in ConPty mode as soon as `VtIo::Initialize` is called (this is during `ConsoleCreateIoThread`, which is right at the end of `ConsoleEstablishHandoff`, which is before the API server starts to process the client connect message). THEORETICALLY, `VtIo` could `Initialize` then fail to create objects in `CreateIoHandlers` (which is what we used to treat as the moment that we were in conpty mode). However, if we do fail out of `CreateIoHandlers`, then the console itself will fail to start up, and just die. So I don't think that's needed. This fixes #15245. I think this is PROBABLY also the solution to #14512, but I'm not gonna explicitly mark closed. We'll loop back on it.
We need to act like a ConPTY just a little earlier in startup. My relevant notes start here: #15245 (comment). Basically, we'd create the first screen buffer with 9001 rows, because it would be created _before_ VtIo would be in a state to say "yes, we're a conpty". Then, if a CLI app emits an entire screenful of text _before_ the terminal has a chance to resize the conpty, then the conpty will explode during `_DoLineFeed`. That method is absolutely not expecting the buffer to get resized (and the old text buffer deallocated). Instead, this will treat the console as in ConPty mode as soon as `VtIo::Initialize` is called (this is during `ConsoleCreateIoThread`, which is right at the end of `ConsoleEstablishHandoff`, which is before the API server starts to process the client connect message). THEORETICALLY, `VtIo` could `Initialize` then fail to create objects in `CreateIoHandlers` (which is what we used to treat as the moment that we were in conpty mode). However, if we do fail out of `CreateIoHandlers`, then the console itself will fail to start up, and just die. So I don't think that's needed. This fixes #15245. I think this is PROBABLY also the solution to #14512, but I'm not gonna explicitly mark closed. We'll loop back on it. (cherry picked from commit 6ad8cd0) Service-Card-Id: 89112504 Service-Version: 1.17
"windowingBehavior": "useAnyExisting",
.bat
in file explorerOther bat files are fine?
It also doesn't crash if you run it manually?
The text was updated successfully, but these errors were encountered: