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

Pasting (or sending text) in terminal can scramble the input #127699

Closed
elspv opened this issue Jun 30, 2021 · 15 comments
Closed

Pasting (or sending text) in terminal can scramble the input #127699

elspv opened this issue Jun 30, 2021 · 15 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug confirmation-pending info-needed Issue requires more information from poster terminal-input Relating to typing in the terminal not doing the right thing, IMEs not working, etc. windows VS Code on Windows issues
Milestone

Comments

@elspv
Copy link

elspv commented Jun 30, 2021

Issue Type: Bug

Text copied to an integrated terminal tab configured to use Cygwin bash is sometimes scrambled. I have observed this both when launching a debug task that copies a command line to a shell and manually pasting from the clipboard.

I can reproduce this problem as follows.

Copy the 60 character string "echo 56789b123456789c123456789d123456789e123456789f123456789" then paste repeatedly into a terminal tab running Cygwin bash.

First attempt:

192.168.3.220:~> echo 56789b123456789c123456789d123456789e123456789f123456789
56789b123456789c123456789d123456789e123456789f123456789

That worked. Second attempt:

192.168.3.220:~> f123456789echo 56789b123456789c123456789d123456789e123456789

That failed. The failure frequency I experience is about 1 in 10.

Notice that the pasted text in the second case was reordered, with the first 50 characters rotated to the end. That 50 character granularity is apparent in every failure I have examined, including the longer strings generated by debug task launches. Checking the source, I see MAX_WRITE_CHECK_SIZE = 50 in terminalProcess.ts in code addressing a similar issue, likely related to my 50 character observation.

In case it matters, the version of bash I'm running is "4.4.12(3)-release" and cygwin.dll version is "3.2.0(0.340/5/3)".

VS Code version: Code 1.57.1 (507ce72, 2021-06-17T13:28:07.755Z)
OS version: Windows_NT x64 10.0.19042
Restricted Mode: No

System Info
Item Value
CPUs Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz (8 x 3408)
GPU Status 2d_canvas: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
skia_renderer: enabled_on
video_decode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) undefined
Memory (System) 31.90GB (26.28GB free)
Process Argv --disable-extensions
Screen Reader no
VM 0%
Extensions disabled
@elspv
Copy link
Author

elspv commented Jul 1, 2021

I should have included one more thing, my terminal profile:

    "terminal.integrated.profiles.windows": {
        "Cygwin": {
            "path": "C:\\cygwin64\\bin\\bash.exe",
            "args":[ "-l" ],
            "env": { "CHERE_INVOKING": "1" }
        }
    },

@Tyriar
Copy link
Member

Tyriar commented Jul 6, 2021

Thanks for the write up, chances are this is indeed related to WRITE_MAX_CHUNK_SIZE and the chunks not being adequately spaced out:

input(data: string, isBinary?: boolean): void {
if (this._isDisposed || !this._ptyProcess) {
return;
}
for (let i = 0; i <= Math.floor(data.length / WRITE_MAX_CHUNK_SIZE); i++) {
const obj = {
isBinary: isBinary || false,
data: data.substr(i * WRITE_MAX_CHUNK_SIZE, WRITE_MAX_CHUNK_SIZE)
};
this._writeQueue.push(obj);
}
this._startWrite();
}

// Queue the next write
this._writeTimeout = setTimeout(() => {
this._writeTimeout = undefined;
this._startWrite();
}, WRITE_INTERVAL_MS);

This is a shame as it's already a hack and has problems that pastes can take quite a while when large. This is where the write chunk mechanism was added: d92c1a8

Not sure what we should do here as I've received bug reports about pastes taking too long and increasing that is just a hack on a hack.

@Tyriar Tyriar added terminal Integrated terminal issues under-discussion Issue is under discussion for relevance, priority, approach windows VS Code on Windows issues labels Jul 6, 2021
@Tyriar Tyriar added this to the Backlog milestone Jul 6, 2021
@elspv
Copy link
Author

elspv commented Jul 6, 2021

Tracking down and eliminating the race condition that allows out of order writes would be ideal. Then the write delay could be eliminated entirely.

@Tyriar
Copy link
Member

Tyriar commented Jul 7, 2021

We just write to a named pipe though, so the race would be in some upstream component outside this project.

@jpambrun
Copy link

jpambrun commented Oct 8, 2021

This is a very serious problem. It's just a question of time before some one paste rm -Rf someFolder/ and nuke his whole system or some similarly devastating. I end up with garbled commands on a daily basis because of this bug. I am using msys2.

@Tyriar Tyriar changed the title Terminal input scrambled Pasting in terminal can scramble the pasted string Oct 19, 2021
@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug upstream Issue identified as 'upstream' component related (exists outside of VS Code) and removed under-discussion Issue is under discussion for relevance, priority, approach upstream Issue identified as 'upstream' component related (exists outside of VS Code) labels Oct 19, 2021
@Tyriar Tyriar changed the title Pasting in terminal can scramble the pasted string Pasting (or sending text) in terminal can scramble the input Oct 19, 2021
@Tyriar Tyriar added terminal-input Relating to typing in the terminal not doing the right thing, IMEs not working, etc. and removed terminal Integrated terminal issues labels Oct 19, 2021
@rdbisme
Copy link

rdbisme commented May 11, 2022

I think I started to get this after I recently update Git Bash using scoop. I tried to use the UCRT64 terminal from MSYS2, and it doesn't do this. I think it's related to MINGW64 specifically?

@DarrenLevine
Copy link

My entire team and I are also seeing this issue in the integrated terminal on windows when sending command sequences to the terminal via several different independent methods. The issue seems to occur more frequently when larger amounts of text are sent to the terminal. And the issue can always be fixed temporarily be closing the offending terminal instance, at which time when sending a new command causes a new terminal to be created, the command will not error out on this first command, but every command sent to that terminal afterwards is susceptible to this bug, at seemingly random intervals.

I'm able to independently reproduce the issue using any of the following extensions to send text to the integrated terminal:

formulahendry.code-runner

malkomalko.send-to-terminal

discretegames.f5anything

I'm running a MSYS2 bash terminal on windows 10 Enterprise and Windows 11. In order to reproduce, I just send a large echo command "echo 1233456..." with a few hundred characters, and after a few back to back runs, the corruptions start appearing.

Also, turning off gpu accelerating seems to reduce the frequency of the issue very slightly, but does not fully fix/prevent it when large amounts of text are sent to the terminal.

@Tyriar
Copy link
Member

Tyriar commented May 31, 2022

Also, turning off gpu accelerating seems to reduce the frequency of the issue very slightly

I doubt that's related to the problem, if anything turning off GPU acceleration consumes more CPU time which could sloww the text events getting sent to the shell.

@wedgberto
Copy link

wedgberto commented Jul 27, 2022

I've been browsing around in the terminal classes and found some potentially guilty uses of setTimeout .

TerminalProcess._doWrite can be called by TerminalProcess._startWrite() asynchronously via a setTimeout

private _startWrite(): void {
// Don't write if it's already queued of is there is nothing to write
if (this._writeTimeout !== undefined || this._writeQueue.length === 0) {
return;
}
this._doWrite();
// Don't queue more writes if the queue is empty
if (this._writeQueue.length === 0) {
this._writeTimeout = undefined;
return;
}
// Queue the next write
this._writeTimeout = setTimeout(() => {
this._writeTimeout = undefined;
this._startWrite();
}, Constants.WriteInterval);
}
I think this is probably not the cause though because the trace logs show IPty#write messages as would be expected (see below)
TerminalDataBufferer.startBuffering also has a potentially responsible asynchronous setTimeout
startBuffering(id: number, event: Event<string | IProcessDataEvent>, throttleBy: number = 5): IDisposable {
const disposable = event((e: string | IProcessDataEvent) => {
const data = (typeof e === 'string' ? e : e.data);
let buffer = this._terminalBufferMap.get(id);
if (buffer) {
buffer.data.push(data);
return;
}
const timeoutId = setTimeout(() => this.flushBuffer(id), throttleBy);
buffer = {
data: [data],
timeoutId: timeoutId,
dispose: () => {
clearTimeout(timeoutId);
this.flushBuffer(id);
disposable.dispose();
}
};
this._terminalBufferMap.set(id, buffer);
});
return disposable;
}
This is more likely closer to being the culprit because it is responsible for the data that is passed to TerminalProcess' IPty.onData whos "IPty#onData" trace logs show the corrupted text that was sent to the terminal when I was reproducing the issue.
ptyProcess.onData(data => {
// Handle flow control
this._unacknowledgedCharCount += data.length;
if (!this._isPtyPaused && this._unacknowledgedCharCount > FlowControlConstants.HighWatermarkChars) {
this._logService.trace(`Flow control: Pause (${this._unacknowledgedCharCount} > ${FlowControlConstants.HighWatermarkChars})`);
this._isPtyPaused = true;
ptyProcess.pause();
}
// Refire the data event
this._logService.trace('IPty#onData', data);
this._onProcessData.fire(data);
if (this._closeTimeout) {
this._queueProcessExit();
}
this._windowsShellHelper?.checkShell();
this._childProcessMonitor?.handleOutput();
});
I have included the vscode trace logs that were created when I ran a jest test using the vscode-jest-runner extension. It works by building a jest command line and sending it to the terminal for execution, but as you can see, a spurious letter 'n' in this instance meant that the command did not run image

@Tyriar
Copy link
Member

Tyriar commented Dec 9, 2022

Thanks @wedgberto for looking into this, since the IPty# logs are all called immediately before handing off to node-pty, this much be an issue in that lib. #write happens when we call into it with the data, #onData is the data we get back.

@Tyriar
Copy link
Member

Tyriar commented Dec 9, 2022

@wedgberto can you confirm whether you have terminal.integrated.windowsEnableConpty set and to what?

@Tyriar Tyriar added confirmation-pending info-needed Issue requires more information from poster labels Dec 9, 2022
@VSCodeTriageBot
Copy link
Collaborator

This issue has been closed automatically because it needs more information and has not had recent activity. See also our issue reporting guidelines.

Happy Coding!

@VSCodeTriageBot VSCodeTriageBot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 17, 2022
@boltex
Copy link

boltex commented Jan 20, 2023

I also have this issue sporadically, with bash under windows.
Version: 1.74.3 (user setup)
Commit: 97dec17
Date: 2023-01-09T16:59:02.252Z
Electron: 19.1.8
Chromium: 102.0.5005.167
Node.js: 16.14.2
V8: 10.2.154.15-electron.0
OS: Windows_NT x64 10.0.19044
Sandboxed: No

@boltex
Copy link

boltex commented Jan 20, 2023

@Tyriar I had terminal.integrated.windowsEnableConpty set, btw.

@boltex
Copy link

boltex commented Jan 20, 2023

Same problem without windowsEnableConpty : Here'S an eample (the lines were pasted from the same clipboard and should be the same , but a 'n' shifted place while pasting!)
image

@github-actions github-actions bot locked and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug confirmation-pending info-needed Issue requires more information from poster terminal-input Relating to typing in the terminal not doing the right thing, IMEs not working, etc. windows VS Code on Windows issues
Projects
None yet
Development

No branches or pull requests

10 participants