Add disposeHandlers

These functions assist with object disposal. They add custom behaviour
during disposal. The primary usage of disposalHandlers is during testing
where some objects should not be disposed in order to avoid testing
errors.

Additionally, move DisposableObject to the pure folder and create unit
tests for it.

Also, add `--disable-gpu` to command line options when running tests.
It helps to avoid error messages like this:

```- [19141:19141:0425/011526.129520:ERROR:sandbox_linux.cc(374)] InitializeSandbox() called with multiple threads in process gpu-process.```

See also https://askubuntu.com/a/1288969

extensions/ql-vscode/src/astViewer.ts

@@ -19,7 +19,8 @@ import { UrlValue, BqrsId } from './pure/bqrs-cli-types';
 import { showLocation } from './interface-utils';
 import { isStringLoc, isWholeFileLoc, isLineColumnLoc } from './pure/bqrs-utils';
 import { commandRunner } from './commandRunner';
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
+import { showAndLogErrorMessage } from './helpers';
 
 export interface AstItem {
   id: BqrsId;
@@ -129,8 +130,13 @@ export class AstViewer extends DisposableObject {
     this.treeDataProvider.db = db;
     this.treeDataProvider.refresh();
     this.treeView.message = `AST for ${path.basename(fileName)}`;
-    this.treeView.reveal(roots[0], { focus: false });
     this.currentFile = fileName;
+    // Handle error on reveal. This could happen if
+    // the tree view is disposed during the reveal.
+    this.treeView.reveal(roots[0], { focus: false })?.then(
+      () => { /**/ },
+      err => showAndLogErrorMessage(err)
+    );
   }
 
   private updateTreeSelection(e: TextEditorSelectionChangeEvent) {
@@ -178,7 +184,12 @@ export class AstViewer extends DisposableObject {
 
       const targetItem = findBest(range, this.treeDataProvider.roots);
       if (targetItem) {
-        this.treeView.reveal(targetItem);
+        // Handle error on reveal. This could happen if
+        // the tree view is disposed during the reveal.
+        this.treeView.reveal(targetItem)?.then(
+          () => { /**/ },
+          err => showAndLogErrorMessage(err)
+        );
       }
     }
   }

extensions/ql-vscode/src/compare/compare-interface.ts

@@ -1,4 +1,4 @@
-import { DisposableObject } from '../vscode-utils/disposable-object';
+import { DisposableObject } from '../pure/disposable-object';
 import {
   WebviewPanel,
   ExtensionContext,

extensions/ql-vscode/src/config.ts

@@ -1,4 +1,4 @@
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 import { workspace, Event, EventEmitter, ConfigurationChangeEvent, ConfigurationTarget } from 'vscode';
 import { DistributionManager } from './distribution';
 import { logger } from './logging';

extensions/ql-vscode/src/databases-ui.ts

@@ -1,5 +1,5 @@
 import * as path from 'path';
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 import {
   Event,
   EventEmitter,

extensions/ql-vscode/src/databases.ts

@@ -15,7 +15,7 @@ import {
   withProgress
 } from './commandRunner';
 import { zipArchiveScheme, encodeArchiveBasePath, decodeSourceArchiveUri, encodeSourceArchiveUri } from './archive-filesystem-provider';
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 import { Logger, logger } from './logging';
 import { registerDatabases, Dataset, deregisterDatabases } from './pure/messages';
 import { QueryServerClient } from './queryserver-client';

extensions/ql-vscode/src/discovery.ts

@@ -1,4 +1,4 @@
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 import { logger } from './logging';
 
 /**

extensions/ql-vscode/src/interface.ts

@@ -1,6 +1,6 @@
 import * as path from 'path';
 import * as Sarif from 'sarif';
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 import * as vscode from 'vscode';
 import {
   Diagnostic,

extensions/ql-vscode/src/logging.ts

@@ -1,5 +1,5 @@
 import { window as Window, OutputChannel, Progress, Disposable } from 'vscode';
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 import * as fs from 'fs-extra';
 import * as path from 'path';
 

extensions/ql-vscode/src/pure/disposable-object.ts

@@ -1,4 +1,11 @@
-import { Disposable } from 'vscode';
+
+// Avoid explicitly referencing Disposable type in vscode.
+// This file cannot have dependencies on the vscode API.
+interface Disposable {
+  dispose(): any;
+}
+
+export type DisposeHandler = (disposable: Disposable) => void;
 
 /**
  * Base class to make it easier to implement a `Disposable` that owns other disposable object.
@@ -40,21 +47,39 @@ export abstract class DisposableObject implements Disposable {
    * @param obj The object to stop tracking.
    */
   protected disposeAndStopTracking(obj: Disposable): void {
-    if (obj !== undefined) {
-      this.tracked!.delete(obj);
+    if (obj && this.tracked) {
+      this.tracked.delete(obj);
       obj.dispose();
     }
   }
 
-  public dispose() {
+  /**
+   * Dispose this object and all contained objects
+   *
+   * @param disposeHandler An optional dispose handler that gets
+   *      passed each element to dispose. The dispose handler
+   *      can choose how (and if) to dispose the object. The
+   *      primary usage is for tests that should not dispose
+   *      all items of a disposable.
+   */
+  public dispose(disposeHandler?: DisposeHandler) {
     if (this.tracked !== undefined) {
       for (const trackedObject of this.tracked.values()) {
-        trackedObject.dispose();
+        if (disposeHandler) {
+          disposeHandler(trackedObject);
+        } else {
+          trackedObject.dispose();
+        }
       }
       this.tracked = undefined;
     }
     while (this.disposables.length > 0) {
-      this.disposables.pop()!.dispose();
+      const disposable = this.disposables.pop()!;
+      if (disposeHandler) {
+        disposeHandler(disposable);
+      } else {
+        disposable.dispose();
+      }
     }
   }
 }

extensions/ql-vscode/src/query-history.ts

@@ -13,7 +13,7 @@ import {
 import { logger } from './logging';
 import { URLSearchParams } from 'url';
 import { QueryServerClient } from './queryserver-client';
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 import { commandRunner } from './commandRunner';
 
 /**

extensions/ql-vscode/src/queryserver-client.ts

@@ -1,6 +1,6 @@
 import * as cp from 'child_process';
 import * as path from 'path';
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 import { Disposable, CancellationToken, commands } from 'vscode';
 import { createMessageConnection, MessageConnection, RequestType } from 'vscode-jsonrpc';
 import * as cli from './cli';

extensions/ql-vscode/src/status-bar.ts

@@ -1,7 +1,7 @@
 import { ConfigurationChangeEvent, StatusBarAlignment, StatusBarItem, window, workspace } from 'vscode';
 import { CodeQLCliServer } from './cli';
 import { CANARY_FEATURES, DistributionConfigListener } from './config';
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 
 /**
  * Creates and manages a status bar item for codeql. THis item contains

extensions/ql-vscode/src/test-adapter.ts

@@ -15,7 +15,7 @@ import {
 import { TestAdapterRegistrar } from 'vscode-test-adapter-util';
 import { QLTestFile, QLTestNode, QLTestDirectory, QLTestDiscovery } from './qltest-discovery';
 import { Event, EventEmitter, CancellationTokenSource, CancellationToken } from 'vscode';
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 import { CodeQLCliServer } from './cli';
 import { getOnDiskWorkspaceFolders } from './helpers';
 import { testLogger } from './logging';

extensions/ql-vscode/src/test-ui.ts

@@ -13,7 +13,7 @@ import {
 
 import { showAndLogWarningMessage } from './helpers';
 import { TestTreeNode } from './test-tree-node';
-import { DisposableObject } from './vscode-utils/disposable-object';
+import { DisposableObject } from './pure/disposable-object';
 import { UIService } from './vscode-utils/ui-service';
 import { QLTestAdapter, getExpectedFile, getActualFile } from './test-adapter';
 import { logger } from './logging';

extensions/ql-vscode/src/vscode-tests/cli-integration/databases.test.ts

@@ -44,7 +44,6 @@ describe('Databases', function() {
 
   afterEach(() => {
     try {
-      // dispose();
       sandbox.restore();
     } catch (e) {
       fail(e);

extensions/ql-vscode/src/vscode-tests/minimal-workspace/databases.test.ts

@@ -20,6 +20,7 @@ import { registerDatabases } from '../../pure/messages';
 import { ProgressCallback } from '../../commandRunner';
 import { CodeQLCliServer } from '../../cli';
 import { encodeArchiveBasePath, encodeSourceArchiveUri } from '../../archive-filesystem-provider';
+import { testDisposeHandler } from '../test-dispose-handler';
 
 describe('databases', () => {
 
@@ -85,7 +86,7 @@ describe('databases', () => {
 
   afterEach(async () => {
     dir.removeCallback();
-    databaseManager.dispose();
+    databaseManager.dispose(testDisposeHandler);
     sandbox.restore();
   });
 

extensions/ql-vscode/src/vscode-tests/no-workspace/astViewer.test.ts

@@ -7,6 +7,7 @@ import * as yaml from 'js-yaml';
 import { AstViewer, AstItem } from '../../astViewer';
 import { commands, Range } from 'vscode';
 import { DatabaseItem } from '../../databases';
+import { testDisposeHandler } from '../test-dispose-handler';
 
 chai.use(chaiAsPromised);
 const expect = chai.expect;
@@ -31,7 +32,7 @@ describe('AstViewer', () => {
   afterEach(() => {
     sandbox.restore();
     if (viewer) {
-      viewer.dispose();
+      viewer.dispose(testDisposeHandler);
       viewer = undefined;
     }
   });

extensions/ql-vscode/src/vscode-tests/no-workspace/contextual/astBuilder.test.ts

@@ -37,7 +37,6 @@ describe('AstBuilder', () => {
   let mockCli: CodeQLCliServer;
   let overrides: Record<string, object | undefined>;
 
-
   beforeEach(() => {
     mockCli = {
       bqrsDecode: sinon.stub().callsFake((_: string, resultSet: 'nodes' | 'edges' | 'graphProperties') => {

extensions/ql-vscode/src/vscode-tests/no-workspace/databases-ui.test.ts

@@ -7,6 +7,7 @@ import { expect } from 'chai';
 import { Uri } from 'vscode';
 
 import { DatabaseUI } from '../../databases-ui';
+import { testDisposeHandler } from '../test-dispose-handler';
 
 describe('databases-ui', () => {
   describe('fixDbUri', () => {
@@ -89,7 +90,7 @@ describe('databases-ui', () => {
     expect(fs.pathExistsSync(db4)).to.be.false;
     expect(fs.pathExistsSync(db5)).to.be.false;
 
-    databaseUI.dispose();
+    databaseUI.dispose(testDisposeHandler);
   });
 
   function createDatabase(storageDir: string, dbName: string, language: string, extraFile?: string) {

extensions/ql-vscode/src/vscode-tests/no-workspace/helpers.test.ts

@@ -7,7 +7,14 @@ import * as path from 'path';
 import * as fs from 'fs-extra';
 import * as sinon from 'sinon';
 
-import { getInitialQueryContents, InvocationRateLimiter, isLikelyDbLanguageFolder, showBinaryChoiceDialog, showBinaryChoiceWithUrlDialog, showInformationMessageWithAction } from '../../helpers';
+import {
+  getInitialQueryContents,
+  InvocationRateLimiter,
+  isLikelyDbLanguageFolder,
+  showBinaryChoiceDialog,
+  showBinaryChoiceWithUrlDialog,
+  showInformationMessageWithAction
+} from '../../helpers';
 import { reportStreamProgress } from '../../commandRunner';
 import Sinon = require('sinon');
 import { fail } from 'assert';

extensions/ql-vscode/src/vscode-tests/run-integration-tests.ts

@@ -112,18 +112,21 @@ function getLaunchArgs(dir: TestDir) {
   switch (dir) {
     case TestDir.NoWorksspace:
       return [
-        '--disable-extensions'
+        '--disable-extensions',
+        '--disable-gpu'
       ];
 
     case TestDir.MinimalWorksspace:
       return [
         '--disable-extensions',
+        '--disable-gpu',
         path.resolve(__dirname, '../../test/data')
       ];
 
     case TestDir.CliIntegration:
       // CLI integration tests requires a multi-root workspace so that the data and the QL sources are accessible.
       return [
+        '--disable-gpu',
         path.resolve(__dirname, '../../test/data'),
         process.env.TEST_CODEQL_PATH!
       ];

extensions/ql-vscode/src/vscode-tests/test-dispose-handler.ts

@@ -0,0 +1,14 @@
+import { Disposable } from 'vscode';
+import { DisposableObject } from '../pure/disposable-object';
+
+export function testDisposeHandler(disposable: any & Disposable) {
+  if (disposable.onDidExpandElement && disposable.onDidCollapseElement && disposable.reveal) {
+    // This looks like a treeViewer. Don't dispose
+    return;
+  }
+  if (disposable instanceof DisposableObject) {
+    disposable.dispose(testDisposeHandler);
+  } else {
+    disposable.dispose();
+  }
+}

extensions/ql-vscode/src/vscode-utils/multi-file-system-watcher.ts

@@ -1,4 +1,4 @@
-import { DisposableObject } from './disposable-object';
+import { DisposableObject } from '../pure/disposable-object';
 import { EventEmitter, Event, Uri, GlobPattern, workspace } from 'vscode';
 
 /**
@@ -62,4 +62,3 @@ export class MultiFileSystemWatcher extends DisposableObject {
     this._onDidChange.fire(uri);
   }
 }
-

extensions/ql-vscode/src/vscode-utils/ui-service.ts

@@ -1,5 +1,5 @@
 import { TreeDataProvider, window } from 'vscode';
-import { DisposableObject } from './disposable-object';
+import { DisposableObject } from '../pure/disposable-object';
 import { commandRunner } from '../commandRunner';
 
 /**

extensions/ql-vscode/test/pure-tests/disposable-object.test.ts

@@ -0,0 +1,122 @@
+import 'chai';
+import 'chai/register-should';
+import 'sinon-chai';
+import * as sinon from 'sinon';
+import 'mocha';
+
+import { DisposableObject } from '../../src/pure/disposable-object';
+import { expect } from 'chai';
+
+describe('DisposableObject and DisposeHandler', () => {
+
+  let disposable1: { dispose: sinon.SinonSpy };
+  let disposable2: { dispose: sinon.SinonSpy };
+  let disposable3: { dispose: sinon.SinonSpy };
+  let disposable4: { dispose: sinon.SinonSpy };
+  let disposableObject: any;
+  let nestedDisposableObject: any;
+  const sandbox = sinon.createSandbox();
+
+  beforeEach(() => {
+    sandbox.restore();
+    disposable1 = { dispose: sandbox.spy() };
+    disposable2 = { dispose: sandbox.spy() };
+    disposable3 = { dispose: sandbox.spy() };
+    disposable4 = { dispose: sandbox.spy() };
+
+    disposableObject = new MyDisposableObject();
+    nestedDisposableObject = new MyDisposableObject();
+  });
+
+  afterEach(() => {
+    sandbox.restore();
+  });
+
+  it('should dispose tracked and pushed objects', () => {
+    disposableObject.push(disposable1);
+    disposableObject.push(disposable2);
+    disposableObject.track(nestedDisposableObject);
+    nestedDisposableObject.track(disposable3);
+
+    disposableObject.dispose();
+
+    expect(disposable1.dispose).to.have.been.called;
+    expect(disposable2.dispose).to.have.been.called;
+    expect(disposable3.dispose).to.have.been.called;
+
+    // pushed items must be called in reverse order
+    sinon.assert.callOrder(disposable2.dispose, disposable1.dispose);
+
+    // now that disposableObject has been disposed, subsequent disposals are
+    // no-ops
+    disposable1.dispose.resetHistory();
+    disposable2.dispose.resetHistory();
+    disposable3.dispose.resetHistory();
+
+    disposableObject.dispose();
+
+    expect(disposable1.dispose).not.to.have.been.called;
+    expect(disposable2.dispose).not.to.have.been.called;
+    expect(disposable3.dispose).not.to.have.been.called;
+  });
+
+  it('should dispose and stop tracking objects', () => {
+    disposableObject.track(disposable1);
+    disposableObject.disposeAndStopTracking(disposable1);
+
+    expect(disposable1.dispose).to.have.been.called;
+    disposable1.dispose.resetHistory();
+
+    disposableObject.dispose();
+    expect(disposable1.dispose).not.to.have.been.called;
+  });
+
+  it('should avoid disposing an object that is not tracked', () => {
+    disposableObject.push(disposable1);
+    disposableObject.disposeAndStopTracking(disposable1);
+
+    expect(disposable1.dispose).not.to.have.been.called;
+
+    disposableObject.dispose();
+    expect(disposable1.dispose).to.have.been.called;
+  });
+
+  it('ahould use a dispose handler', () => {
+    const handler = (d: any) => (d === disposable1 || d === disposable3 || d === nestedDisposableObject)
+      ? d.dispose(handler)
+      : void (0);
+
+    disposableObject.push(disposable1);
+    disposableObject.push(disposable2);
+    disposableObject.track(nestedDisposableObject);
+    nestedDisposableObject.track(disposable3);
+    nestedDisposableObject.track(disposable4);
+
+    disposableObject.dispose(handler);
+
+    expect(disposable1.dispose).to.have.been.called;
+    expect(disposable2.dispose).not.to.have.been.called;
+    expect(disposable3.dispose).to.have.been.called;
+    expect(disposable4.dispose).not.to.have.been.called;
+
+    // now that disposableObject has been disposed, subsequent disposals are
+    // no-ops
+    disposable1.dispose.resetHistory();
+    disposable2.dispose.resetHistory();
+    disposable3.dispose.resetHistory();
+    disposable4.dispose.resetHistory();
+
+    disposableObject.dispose();
+
+    expect(disposable1.dispose).not.to.have.been.called;
+    expect(disposable2.dispose).not.to.have.been.called;
+    expect(disposable3.dispose).not.to.have.been.called;
+    expect(disposable4.dispose).not.to.have.been.called;
+  });
+
+  class MyDisposableObject extends DisposableObject {
+    constructor() {
+      super();
+    }
+  }
+});