RFC: Subscriptions as "stacks" of teardowns?

[benlesh]

Problem Space

The current TC39 disposal proposal includes a DisposableStack that unfortunately probably doesn't suit our needs for ergonomics, but does include the idea that registered disposals (finalizations) should be fired in a last-in-first-out (LIFO) sort of way. The argument for doing this is that it's reasonable to assume that a resource that you set up that creates a secondary resource would need to be torn down after the secondary resource. For example, if you create a database connection, then a transaction, you wouldn't finalize the transaction before you closed the connection.

Subscription in RxJS, for a very long time have sort of a "mix" of behaviors when it comes to LIFO/FIFO execution of teardowns/finalizers.

Subscription chains — those that are created by chains of operators — are generally executed LIFO due to composition. The consumer subscribes, which creates a subscription, that you then add a subscriptoin to some source to, which then gets a subscription added to it, and so on. When you subscribe to the Observable that is the result of your operator chain, it subscribes to the observable that is the result of the operator for it, which links the subscriptions, where the "downstream" subscription is the "parent" and the "upstream" subscription(s) is/are the child(ren). It repeats this all the way up to the head of the chain, the original source.

When you call unsubscribe on that subscription, it executes whatever logic it already had set up, then it unsubscribes it's children (mostly due to the circumstantial order in which they were added). Those children execute their logic and then unsubscribe their children, and so on. The result is, that at least between operators in RxJS, the execution of teardowns is done in a LIFO manner.

However, when it comes to teardowns have been add-ed to Subscription, those are all executed in the same order in which they were added, making them FIFO.

Basically:

const subscription = new Subscription();
const connection = createDatabaseConn();
subscription.add(() => connection.close());
const transaction = connection.createTransaction();
subscription.add(() => transaction.finalize());

// The connection will be closed before the transaction is finalized.
subscription.unsubscribe();

Now most people would work around this like so:

const subscription = new Subscription();
const connection = createDatabaseConn();
const transaction = connection.createTransaction();
subscription.add(() => {
  transaction.finalize();
  connection.close();
});

// This works a bit better
subscription.unsubscribe();

...but then what if there's an error thrown during connection.createTransaction()? If that's the case, there's a few things you can do:

// This would work in an `Observable` constructor:

const source = new Observable(() => {
  const subscription = new Subscription();
  subscription.add(() => {
    transaction?.finalize();
    connection?.close();
  });
  var connection = createDatabaseConn();
  var transaction = connection.createTransaction();

  // stuff here.
  
  return subscription;
});

or

// Or for more "plain" use of Subscriptions you'd have to do something like this

let connection: Connection | undefined;
let transaction: Tranaction | undefined;
try {
  subscription = new Subscription();
  const connection = createDatabaseConn();
  const transaction = connection.createTransaction();
  subscription.add(() => {
    transaction.finalize();
    connection.close();
  });
} catch (err) {
  transaction?.finalize();
  connection?.close();
  throw err;
}

// This works a bit better
subscription.unsubscribe();

Proposed Solution: Execute Subscription Teardowns LIFO

That is to say this would work as follows:

const subscription = new Subscription();
subscription.add(() => console.log(2))
subscription.add(() => console.log(1))
// Logs 1 then 2
subscription.unsubscribe();

Meaning that the following would just work as expected:

const subscription = new Subscription();
const connection = createDatabaseConn();
subscription.add(() => connection.close());
const transaction = connection.createTransaction();
subscription.add(() => transaction.finalize());

// The connection will be closed before the transaction is finalized.
subscription.unsubscribe();

Counterpoints

The only real counterpoints I have here are:

1. We've lived for YEARS and YEARS and this has never been an issue. Although I suspect that people are just working around it, because the workarounds are trivial.
2. It's a breaking change.
3. It does make it harder to reason about what has been added to a single subscription. However when subscriptions are chained, it might actually make things easier to reason about, because everything would be LIFO at that point, rather than LIFO between chained subscriptions, and FIFO within each subscription's teardowns.

Other thoughts

1. Soon we'll want to implement Symbol.dispose on Subscription (per the stage 3 TC39 proposal)
2. We'll want to support add-ing disposables that have the Symbol.dispose method.
3. We might want to implement a use that supports the Symbol.dispose method, or augment add to return a Disposable instance — Ironically, this is sort of what RxJS 5-6 was doing). Where add(() => void) would return a disposable that would call and remove the function when disposed, add(Disposable) would return the Disposable instance itself (ala DisposableStack)

Thank you to @rbuckton and @bakkot for the ideas here.

[rbuckton]

Regarding this example:

// Or for more "plain" use of Subscriptions you'd have to do something like this

let connection: Connection | undefined;
let transaction: Tranaction | undefined;
try {
  subscription = new Subscription();
  const connection = createDatabaseConn();
  const transaction = connection.createTransaction();
  subscription.add(() => {
    transaction.finalize();
    connection.close();
  });
} catch (err) {
  transaction?.finalize();
  connection?.close();
  throw err;
}

// This works a bit better
subscription.unsubscribe();

Calling transaction.finalize() followed by connection.close() has the potential to leak connections if an error is thrown by transaction.finalize() when you later unsubscribe. A more robust approach would be:

subscripton.add(() => {
  try {
    transaction.finalize();
  }
  finally {
    connection.close();
  }
});

which ensures that the connection is still closed.

One of the benefits of DisposableStack (and using declarations in general) is that those cleanup semantics are handled for you:

const stack = new DisposableStack();
const connection = createDatabaseConn();
stack.defer(() => connection.close());

const transaction = connection.createTransaction();
stack.defer(() => transaction.finalize());

// later...

// this not only ensures that `transaction` is finalized before `connection` is closed, but that
// `connection` will be closed *even if* `transaction.finalize()` throws.
stack.dispose();

[benlesh]

Good point.

Related: I forgot to mention also that errors thrown in subscription finalizations don't prevent the remaining finalizers from being called.