From 616025d4dae258ae7b108d62b11e0c5130a74806 Mon Sep 17 00:00:00 2001
From: Mathieu Hofman <mathieu@agoric.com>
Date: Mon, 1 Nov 2021 02:18:32 +0000
Subject: [PATCH] fix: prevent adding resource after aggregate disposal

---
 README.md           |  4 +---
 async-disposable.js | 18 +++++++++++++++++-
 disposable.js       | 17 ++++++++++++++++-
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/README.md b/README.md
index c181c60..a02e87a 100644
--- a/README.md
+++ b/README.md
@@ -173,7 +173,7 @@ export const AsyncDisposable: AggregateAsyncDisposableConstructor;
 
 ### `using` helper: add resources for tracking
 
-The `AggregateDisposable` and `AggregateAsyncDisposable` objects expose a `using` helper on their instance which can be used to add resources for tracking after construction of the aggregate object. The `using` helper can be detached from the aggregate object (it's bound at construction). It passes through its value for chaining, or assignment at acquisition time. `using` accepts a dispose callback function as an optional argument. This can be used to implement disposal for values which do not implement the disposables interfaces. In that case the value is passed as `this` context to the dispose callback
+The `AggregateDisposable` and `AggregateAsyncDisposable` objects expose a `using` helper on their instance which can be used to add resources for tracking after construction of the aggregate object. The `using` helper can be detached from the aggregate object (it's bound at construction). Calling `using` after the aggregate has been disposed throws an error. It passes through its value for chaining, or assignment at acquisition time. `using` accepts a dispose callback function as an optional argument. This can be used to implement disposal for values which do not implement the disposables interfaces. In that case the value is passed as `this` context to the dispose callback
 
 #### Aggregate `Disposable`
 
@@ -239,8 +239,6 @@ interface AggregateAsyncDisposableUsing {
 }
 ```
 
-The `Disposable`'s `using` helper function can be used to track any _disposable like_ resource. It captures the _disposable_ and its dispose method, or the dispose callback, then passes through the value. Additionally `using` can be called with an `onDispose` callback as second argument, which will be called with the value as `this` context. When the aggregate object is disposed of, the tracked resources are disposed of in reverse order to which they were added.
-
 The `AsyncDisposable`'s `using` helper function can be used to track any _disposable_ or _async disposable like_ resource. It captures the _disposable_ and its dispose method, the _async disposable_ and its async dispose method, or the async dispose callback, then passes through the value. Additionally `using` can be called with an `onDispose` async callback as second argument, which will be called with the value as `this` context. When the aggregate async object is disposed of, the tracked resources are disposed of in reverse order to which they were added.
 
 The disposal of an _async disposable like_ resource is awaited before moving to the next resource. The disposal of a _disposable_ resource is not awaited. The aggregate disposal step is always awaited even if all tracked resources are _disposable_ which are disposed of synchronously.
diff --git a/async-disposable.js b/async-disposable.js
index 4f997cb..b7c7891 100644
--- a/async-disposable.js
+++ b/async-disposable.js
@@ -346,12 +346,28 @@ export const AsyncDisposable = /** @type {DisposableConstructor} */ (
      * @param {DisposeMethod} [onDispose]
      */
     using(value, onDispose) {
-      const stack = this.#resourceStack;
+      const isDisposed = this.#state === "disposed";
+
+      const stack = !isDisposed ? this.#resourceStack : [];
 
       typeof onDispose === "function"
         ? addDisposable(onDispose, value, stack)
         : addDisposable(value, value, stack);
 
+      if (isDisposed) {
+        /** @type {unknown} */
+        let err = new TypeError(
+          "Can't add resource, AsyncDisposable has already been disposed"
+        );
+        try {
+          // For an "async" resource this will result in an unhandled rejection
+          disposeRecord(/** @type {DisposableResourceRecord} */ (stack.pop()));
+        } catch (disposeError) {
+          err = mergeCause(disposeError, err);
+        }
+        throw err;
+      }
+
       return value;
     }
 
diff --git a/disposable.js b/disposable.js
index 9068d39..d3853e3 100644
--- a/disposable.js
+++ b/disposable.js
@@ -316,12 +316,27 @@ export const Disposable = /** @type {DisposableConstructor} */ (
      * @param {DisposeMethod} [onDispose]
      */
     using(value, onDispose) {
-      const stack = this.#resourceStack;
+      const isDisposed = this.#state === "disposed";
+
+      const stack = !isDisposed ? this.#resourceStack : [];
 
       typeof onDispose === "function"
         ? addDisposable(onDispose, value, stack)
         : addDisposable(value, value, stack);
 
+      if (isDisposed) {
+        /** @type {unknown} */
+        let err = new TypeError(
+          "Can't add resource, Disposable has already been disposed"
+        );
+        try {
+          disposeRecord(/** @type {DisposableResourceRecord} */ (stack.pop()));
+        } catch (disposeError) {
+          err = mergeCause(disposeError, err);
+        }
+        throw err;
+      }
+
       return value;
     }