Fixes observable leak

microsoft · Jun 20, 2024 · 34899a7 · 34899a7
1 parent e6c0d82
commit 34899a7
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 3 deletions.
diff --git a/src/vs/base/common/observableInternal/derived.ts b/src/vs/base/common/observableInternal/derived.ts
@@ -140,19 +140,28 @@ export function derivedDisposable<T extends IDisposable | undefined>(computeFnOr
  computeFn = computeFnOrUndefined as any;
  }
 
- const store = new DisposableStore();
+ let store: DisposableStore | undefined = undefined;
  return new Derived(
  new DebugNameData(owner, undefined, computeFn),
  r => {
- store.clear();
+ if (!store) {
+ store = new DisposableStore();
+ } else {
+ store.clear();
+ }
  const result = computeFn(r);
  if (result) {
  store.add(result);
  }
  return result;
  }, undefined,
  undefined,
- () => store.dispose(),
+ () => {
+ if (store) {
+ store.dispose();
+ store = undefined;
+ }
+ },
  strictEquals
  );
 }

diff --git a/src/vs/base/test/common/observable.test.ts b/src/vs/base/test/common/observable.test.ts
@@ -5,8 +5,10 @@
 
 import assert from 'assert';
 import { Emitter, Event } from 'vs/base/common/event';
+import { DisposableStore } from 'vs/base/common/lifecycle';
 import { ISettableObservable, autorun, derived, ITransaction, observableFromEvent, observableValue, transaction, keepObserved, waitForState, autorunHandleChanges, observableSignal } from 'vs/base/common/observable';
 import { BaseObservable, IObservable, IObserver } from 'vs/base/common/observableInternal/base';
+import { derivedDisposable } from 'vs/base/common/observableInternal/derived';
 import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
 
 suite('observables', () => {
@@ -1236,6 +1238,35 @@ suite('observables', () => {
  'rejected {\"state\":\"error\"}'
  ]);
  });
+
+ test('derived as lazy', () => {
+ const store = new DisposableStore();
+ const log = new Log();
+ let i = 0;
+ const d = derivedDisposable(() => {
+ const id = i++;
+ log.log('myDerived ' + id);
+ return {
+ dispose: () => log.log(`disposed ${id}`)
+ };
+ });
+
+ d.get();
+ assert.deepStrictEqual(log.getAndClearEntries(), ['myDerived 0', 'disposed 0']);
+ d.get();
+ assert.deepStrictEqual(log.getAndClearEntries(), ['myDerived 1', 'disposed 1']);
+
+ d.keepObserved(store);
+ assert.deepStrictEqual(log.getAndClearEntries(), []);
+ d.get();
+ assert.deepStrictEqual(log.getAndClearEntries(), ['myDerived 2']);
+ d.get();
+ assert.deepStrictEqual(log.getAndClearEntries(), []);
+
+ store.dispose();
+
+ assert.deepStrictEqual(log.getAndClearEntries(), ['disposed 2']);
+ });
  });
 
  test('observableValue', () => {