denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
94.02k stars 5.23k forks source link

WebSocket memory leak #25070

Open sergeysolovev opened 1 month ago

sergeysolovev commented 1 month ago

Version: Deno 1.45.5 OS: Amazon Linux or Ubuntu Linux

Steps: start the server and the client scripts, the server mem should quickly go up, stop the client script to disconnect. Because the server has disconnected the socket I expect the memory eventually to go down, but it keeps high for very long time.

Here is the log in my case:

Screenshot 2024-08-16 at 23 35 58

Server: spam a client with numerous messages containing a long string. Notice I don’t check the socket’s bufferedAmount field.

const LONG_STRING = 'x'.repeat(1024 * 1024);
const handlers: ClientHandler[] = [];

setInterval(() => {
  const mem = (Deno.memoryUsage().rss / 1024 / 1024).toFixed();
  console.log('srv_stat', { mem });
}, 2500);

Deno.serve({ port: 3333 }, (req) => {
  if (req.headers.get('upgrade') != 'websocket') {
    return new Response(null, { status: 501 });
  }

  const wsUpgrade = Deno.upgradeWebSocket(req);
  handlers.push(new ClientHandler(wsUpgrade.socket));

  return wsUpgrade.response;
});

class ClientHandler {
  socket: WebSocket | null;
  isSubscribed = false;

  constructor(socket: WebSocket) {
    this.socket = socket;
    this.socket.addEventListener('close', this.handleClose);
    this.socket.addEventListener('error', this.handleError);
    this.socket.addEventListener('open', this.handleOpen);
    this.socket.addEventListener('message', this.handleMessage);
  }

  close() {
    this.socket?.close();
    this.socket = null;
  }

  async runSendLoop() {
    while (true) {
      if (this.socket === null) break;
      if (this.socket.readyState === WebSocket.CLOSED) break;
      if (this.socket.readyState === WebSocket.CLOSING) break;

      for (let i = 0; i < 10; i++) {
        this.socket.send(LONG_STRING);
      }

      await this.delay(0);
    }

    console.log('closed_send_loop');
  }

  delay(ms: number) {
    return new Promise((resolve) => setTimeout(resolve, ms));
  }

  handleClose = () => {
    console.log('ws_close');
    if (this.socket === null) return;

    this.socket.removeEventListener('close', this.handleClose);
    this.socket.removeEventListener('error', this.handleError);
    this.socket.removeEventListener('open', this.handleOpen);
    this.socket.removeEventListener('message', this.handleMessage);
    this.socket = null;
  };

  handleOpen = () => {
    console.log('ws_open');
  };

  handleError = (_ev: Event) => {
    const errEv = _ev as unknown as ErrorEvent;
    console.log('ws_err', errEv.message);
  };

  handleMessage = (ev: MessageEvent) => {
    const data = ev.data as string;

    if (data === 'subscribe' && !this.isSubscribed) {
      this.isSubscribed = true;
      console.log('subscribed');
      this.runSendLoop();
      return;
    }
  };
}

Deno.addSignalListener('SIGINT', () => {
  for (const handler of handlers) {
    handler.close;
  }
  Deno.exit();
});

Client: very simple, just send a "subscribe” string and keep receiving the messages from the server

class Client {
  socket?: WebSocket;
  msgCount = 0;
  msgTotLen = 0;

  handleOpen = () => {
    console.log('ws_open');
    this.socket!.send('subscribe');
  };

  handleClose = () => {
    console.log('ws_close');
  };

  handleMessage = (ev: MessageEvent) => {
    this.msgCount += 1;
    this.msgTotLen += (ev.data as string).length;
  };

  handleError = (e: Event) => {
    const errEvent = e as unknown as ErrorEvent;
    console.log('ws_err', errEvent.message);
  };

  open() {
    this.socket = new WebSocket('http://localhost:3333');
    this.socket.addEventListener('open', this.handleOpen);
    this.socket.addEventListener('close', this.handleClose);
    this.socket.addEventListener('error', this.handleError);
    this.socket.addEventListener('message', this.handleMessage);
  }

  close() {
    this.socket?.close();
  }
}

const client = new Client();
client.open();

Deno.addSignalListener('SIGINT', () => {
  client.close();
  setTimeout(() => Deno.exit(), 1000);
});

setInterval(() => {
  const mem = (Deno.memoryUsage().rss / 1024 / 1024).toFixed();
  console.log('stats', {
    mem,
    cnt: client.msgCount,
    len: (client.msgTotLen / 1e6).toFixed(1) + 'm',
  });
}, 2500);
littledivy commented 1 month ago

This is expected.

while (true) {
  // ...
  for (let i = 0; i < 10; i++) {
    this.socket.send(LONG_STRING);
  }

  await this.delay(0);
}

The code is in a loop and filling up the write queue with lots of data very quickly.

The WebSocket API by design does not provide async backpressure (WebSocketStream fixes this) so its the caller that needs to check socket.bufferedAmount.

sergeysolovev commented 1 month ago

Hey @littledivy yes I agree with your comment and even mentioned that in my original message. So yes I expect the memory to grow. What I don’t expect is that some (quite big part part) of the allocated memory stays there even after the connection is closed on the both sides.

lucacasonato commented 3 weeks ago

I do not think this is a memory leak. This is V8 keeping around memory until the system needs it for other reasons - unless your program will eventually crash with an OOM, it is not a leak. V8 does not immediately release memory that JS is not currently using to the OS. It keeps it for a while in case JS needs to alloc in that memory space again. It's a relatively important performance optimization.