dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.22k stars 1.57k forks source link

Cancelling the subscription to a WebSocket leads to longer isolate shutdown time #51776

Open abitofevrything opened 1 year ago

abitofevrything commented 1 year ago

Calling cancel() on a subscription to a WebSocket - before or after close() is called on the same websocket - makes the isolate containing the websocket stay alive for an extra 5 seconds after the websocket & subscription are closed & cancelled (assuming the connection is the only thing keeping the isolate alive).

To reproduce:

import 'dart:async';
import 'dart:io';
import 'dart:isolate';

void cancelBeforeClose(SendPort sendPort) async {
  final connection = await WebSocket.connect('wss://socketsbay.com/wss/v2/1/demo/');
  final subscription = connection.listen((event) {});

  sendPort.send(null);

  await subscription.cancel();
  await connection.close();

  sendPort.send(null);
}

void cancelAfterClose(SendPort sendPort) async {
  final connection = await WebSocket.connect('wss://socketsbay.com/wss/v2/1/demo/');
  final subscription = connection.listen((event) {});

  sendPort.send(null);

  await connection.close();
  await subscription.cancel();

  sendPort.send(null);
}

void onlyClose(SendPort sendPort) async {
  final connection = await WebSocket.connect('wss://socketsbay.com/wss/v2/1/demo/');
  connection.listen((event) {});

  sendPort.send(null);

  await connection.close();

  sendPort.send(null);
}

Future<void> test(String name, void Function(SendPort) isolateMain) async {
  final startDisconnectionNotifier = ReceivePort();
  final isolateExitNotifier = ReceivePort();

  Stopwatch? timer;
  late Duration timeToClose;

  startDisconnectionNotifier.listen((_) {
    if (timer == null) {
      timer = Stopwatch()..start();
    } else {
      timeToClose = timer!.elapsed;
    }
  });

  isolateExitNotifier.listen((_) {
    timer!.stop();

    print('$name: took $timeToClose to close and ${timer!.elapsed} to exit');

    startDisconnectionNotifier.close();
    isolateExitNotifier.close();
  });

  await Isolate.spawn(
    isolateMain,
    startDisconnectionNotifier.sendPort,
    onExit: isolateExitNotifier.sendPort,
  );
}

void main() async {
  test('cancelBeforeClose', cancelBeforeClose);
  test('cancelAfterClose', cancelAfterClose);
  test('onlyClose', onlyClose);
}

Output:

onlyClose: took 0:00:00.019732 to close and 0:00:00.087347 to exit
cancelAfterClose: took 0:00:00.020784 to close and 0:00:05.001326 to exit
cancelBeforeClose: took 0:00:00.021092 to close and 0:00:05.007623 to exit
mraleph commented 1 year ago

/cc @brianquinlan

SandPod commented 4 months ago

I seem to have stumbled upon something related when trying to close web socket connections actively and noticed this line in the close implementation of websockets: https://github.com/dart-lang/sdk/blob/0c9067c626f62b953bd8761628a46320b693716f/sdk/lib/_http/websocket_impl.dart#L1255

For my knowledge:

  1. Why is a 5-second delay required here?
  2. Why is this 5-second delay not included in the await of the close method?