Skip to content

Commit

Permalink
Merge pull request #4160 from jerch/tweak_serverside_buffering
Browse files Browse the repository at this point in the history
tweak buffering in server.js
  • Loading branch information
Tyriar authored Oct 1, 2022
2 parents 969db2b + 150499b commit e3e3c34
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 21 deletions.
50 changes: 30 additions & 20 deletions demo/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ function startServer() {
var app = express();
expressWs(app);

var terminals = {},
logs = {};
var terminals = {};

app.use('/xterm.css', express.static(__dirname + '/../css/xterm.css'));
app.get('/logo.png', (req, res) => {
Expand Down Expand Up @@ -55,10 +54,6 @@ function startServer() {

console.log('Created terminal with PID: ' + term.pid);
terminals[term.pid] = term;
logs[term.pid] = '';
term.on('data', function(data) {
logs[term.pid] += data;
});
res.send(term.pid.toString());
res.end();
});
Expand All @@ -77,16 +72,26 @@ function startServer() {
app.ws('/terminals/:pid', function (ws, req) {
var term = terminals[parseInt(req.params.pid)];
console.log('Connected to terminal ' + term.pid);
ws.send(logs[term.pid]);

// unbuffered delivery after user input
let userInput = false;

// string message buffering
function buffer(socket, timeout) {
function buffer(socket, timeout, maxSize) {
let s = '';
let sender = null;
return (data) => {
s += data;
if (!sender) {
sender = queueMicrotask(() => {
if (s.length > maxSize || userInput) {
userInput = false;
socket.send(s);
s = '';
if (sender) {
clearTimeout(sender);
sender = null;
}
} else if (!sender) {
sender = setTimeout(() => {
socket.send(s);
s = '';
sender = null;
Expand All @@ -95,15 +100,24 @@ function startServer() {
};
}
// binary message buffering
function bufferUtf8(socket, timeout) {
function bufferUtf8(socket, timeout, maxSize) {
let buffer = [];
let sender = null;
let length = 0;
return (data) => {
buffer.push(data);
length += data.length;
if (!sender) {
sender = queueMicrotask(() => {
if (length > maxSize || userInput) {
userInput = false;
socket.send(Buffer.concat(buffer, length));
buffer = [];
length = 0;
if (sender) {
clearTimeout(sender);
sender = null;
}
} else if (!sender) {
sender = setTimeout(() => {
socket.send(Buffer.concat(buffer, length));
buffer = [];
sender = null;
Expand All @@ -112,27 +126,23 @@ function startServer() {
}
};
}
const send = USE_BINARY ? bufferUtf8(ws, 5) : buffer(ws, 5);
const send = (USE_BINARY ? bufferUtf8 : buffer)(ws, 5, 262144);

// WARNING: This is a naive implementation that will not throttle the flow of data. This means
// it could flood the communication channel and make the terminal unresponsive. Learn more about
// the problem and how to implement flow control at https://xtermjs.org/docs/guides/flowcontrol/
term.on('data', function(data) {
try {
send(data);
} catch (ex) {
// The WebSocket is not open, ignore
}
send(data);
});
ws.on('message', function(msg) {
term.write(msg);
userInput = true;
});
ws.on('close', function () {
term.kill();
console.log('Closed terminal ' + term.pid);
// Clean things up
delete terminals[term.pid];
delete logs[term.pid];
});
});

Expand Down
2 changes: 1 addition & 1 deletion src/common/input/WriteBuffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export class WriteBuffer {
this._bufferOffset = 0;

// If this is the first write call after the user has done some input,
// parse it immediately in an upcoming microtask to minimize reduce input,
// parse it immediately to minimize input latency,
// otherwise schedule for the next event
if (this._didUserInput) {
this._didUserInput = false;
Expand Down

0 comments on commit e3e3c34

Please sign in to comment.