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

Flow control using XON/XOFF #3171

Closed
tandatle opened this issue Nov 10, 2020 · 3 comments
Closed

Flow control using XON/XOFF #3171

tandatle opened this issue Nov 10, 2020 · 3 comments
Labels
type/question A question on how to use the library

Comments

@tandatle
Copy link

Hi contributors,

I am using xtermjs to build a terminal application running on web browser, and the target server uses Linux. I noticed that xtermjs v4.9.0 is not responsive to ^C when running yes command.
Then I backed to v4.0.2 and enabled useFlowControl option, it worked.

The useFlowControl using XON/XOFF was added from v2.3.0 (PR #447) but removed from v4.1.0 (PR #2422).
I can't find any discussion on the reason to remove it. Does XON/XOFF solution have any pitfall?

If not, can we have a plan to bring back this feature to the lib or I should implement it on my own application?

The flow control described in document (https://xtermjs.org/docs/guides/flowcontrol/) does not help much since I use websocket to connect to backend.

@mofux
Copy link
Contributor

mofux commented Nov 10, 2020

We removed XON / XOFF because it didn't work reliable in some scenarios. From the back of my head I remember we had problems with the ZSH shell, which used the XON / XOFF sequences for something different. I believe the Windows Terminal also had problems with these sequences.

Anyway - the way I solved this (also using WebSockets - in my case socket.io but it doesn't matter) is to manually establish a flow control mechanism over the socket itself.

So - whenever data arrives on the socket that should be written to xterm.js, we increase a counter, and once the data was successfully written to xterm.js (which means it has been rendered by xterm.js), we decrease that counter.

If the counter exceeds a certain threshold, we send a pause message over the Websocket, which on the server side will pause the pty stream (terminalStream.pause()). Once we go below that threshold, we send a resume message over the Websocket, which on the server side will resume the pty stream (terminalStream.resume()).

Note that the terminalStream.pause() and terminalStream.resume() methods are the built-in node.js Stream methods for handling backpressure. node-pty as well as ssh2 support backpressure handling via these methods.

// client
const MAX_PENDING_WRITES = 5;
let pendingWrites = 0;
let paused = false;
socket.on('data', (data) => {
  pendingWrites++;
  xterm.write(data, () => {
    pendingWrites--;
    if (pendingWrites > MAX_PENDING_WRITES && !paused) {
      paused = true;
      socket.emit('pause');
      return;
    }
    if (pendingWrites <= MAX_PENDING_WRITES && paused) {
      paused = false;
      socket.emit('resume');
      return;
    }
  });
});
// server
terminalStream.on('data', (data) => socket.emit('data', data);
socket.on('data', (data) => terminalStream.write(data));
socket.on('pause', () => terminalStream.pause();
socket.on('resume', () => terminalStream.resume();

This mechanism is working pretty reliable so far. The yes command won't flood the connection anymore. There is still a delay of about 0.5s between CTRL+C and the actual termination of the command in high throughput scenarios, but IMO that is okay.

I hope it helps.

@jerch
Copy link
Member

jerch commented Nov 10, 2020

@tandatle Also see the docs about flowcontrol.

Edit: Sorry, overlooked your docs remark. Care to explain whats unclear there? Yes the websocket section is just a stub (no ready-to-go snippet), as I did not what to create tons of snippets for different socket libs and custom needs (this is hard to get done right with websockets for security reasons alone). Still feel free to add a concrete snippet, that covers those aspects.

@Tyriar Tyriar added the type/documentation Regarding website, API documentation, README, etc. label Nov 10, 2020
@tandatle
Copy link
Author

@mofux @jerch Thanks for your helps.
I am not using node.js at the backend, but a process written in C++, so I may have to do some more investigations to choose between using XON/XOFF and implementing backpresure handling in my backend process.
Thank you so much.

@Tyriar Tyriar closed this as completed Nov 11, 2020
@Tyriar Tyriar added type/question A question on how to use the library and removed type/documentation Regarding website, API documentation, README, etc. labels Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/question A question on how to use the library
Projects
None yet
Development

No branches or pull requests

4 participants