test(NODE-5028): assert MongoClients are garbage collectable (#3553)

Author: nbbeeken

package-lock.json

@@ -94,6 +94,7 @@
     "tsd": "^0.25.0",
     "typescript": "^4.9.4",
     "typescript-cached-transpile": "^0.0.6",
+    "v8-heapsnapshot": "^1.2.0",
     "xml2js": "^0.4.23",
     "yargs": "^17.6.0"
   },

package.json

@@ -0,0 +1,89 @@
+import { expect } from 'chai';
+
+import { runScript } from './resource_tracking_script_builder';
+
+/**
+ * This 5MB range is selected arbitrarily and should likely be raised if failures are seen intermittently.
+ *
+ * The goal here is to catch unbounded memory growth, currently runScript defaults to 100 iterations of whatever test code is passed in
+ * More than a 5MB growth in memory usage after the script has finished and _should have_ cleaned up all its resources likely indicates that
+ * the growth will continue if the script is changed to iterate the the test code more.
+ */
+const MB_PERMITTED_OFFSET = 5;
+
+describe('Driver Resources', () => {
+  let startingMemoryUsed;
+  let endingMemoryUsed;
+  let heap;
+
+  beforeEach(function () {
+    if (globalThis.AbortController == null) {
+      if (this.currentTest) this.currentTest.skipReason = 'Need AbortController to run this test';
+      this.currentTest?.skip();
+    }
+    if (typeof this.configuration.serverApi === 'string') {
+      if (this.currentTest) {
+        this.currentTest.skipReason = 'runScript does not support serverApi settings';
+      }
+      this.currentTest?.skip();
+    }
+  });
+
+  context('on MongoClient.close()', () => {
+    before('create leak reproduction script', async function () {
+      if (globalThis.AbortController == null || typeof this.configuration.serverApi === 'string') {
+        return;
+      }
+      try {
+        const res = await runScript(
+          'no_resource_leak_connect_close',
+          this.configuration,
+          async function run({ MongoClient, uri }) {
+            const mongoClient = new MongoClient(uri, { minPoolSize: 100 });
+            await mongoClient.connect();
+            // Any operations will reproduce the issue found in v5.0.0/v4.13.0
+            // it would seem the MessageStream has to be used?
+            await mongoClient.db().command({ ping: 1 });
+            await mongoClient.close();
+          }
+        );
+        startingMemoryUsed = res.startingMemoryUsed;
+        endingMemoryUsed = res.endingMemoryUsed;
+        heap = res.heap;
+      } catch (error) {
+        // We don't expect the process execution to ever fail,
+        // leaving helpful debugging if we see this in CI
+        console.log(`runScript error message: ${error.message}`);
+        console.log(`runScript error stack: ${error.stack}`);
+        console.log(`runScript error cause: ${error.cause}`);
+        // Fail the test
+        this.test?.error(error);
+      }
+    });
+
+    describe('ending memory usage', () => {
+      it(`is within ${MB_PERMITTED_OFFSET}MB of starting amount`, async () => {
+        // Why check the lower bound? No reason, but it would be very surprising if we managed to free MB_PERMITTED_OFFSET MB of memory
+        // I expect us to **never** be below the lower bound, but I'd want to know if it happened
+        expect(
+          endingMemoryUsed,
+          `script started with ${startingMemoryUsed}MB heap but ended with ${endingMemoryUsed}MB heap used`
+        ).to.be.within(
+          startingMemoryUsed - MB_PERMITTED_OFFSET,
+          startingMemoryUsed + MB_PERMITTED_OFFSET
+        );
+      });
+    });
+
+    describe('ending heap snapshot', () => {
+      it('has 0 MongoClients in memory', async () => {
+        const clients = heap.nodes.filter(n => n.name === 'MongoClient' && n.type === 'object');
+        // lengthOf crashes chai b/c it tries to print out a gigantic diff
+        expect(
+          clients.length,
+          `expected no MongoClients in the heapsnapshot, found ${clients.length}`
+        ).to.equal(0);
+      });
+    });
+  });
+});

test/integration/node-specific/resource_clean_up.test.ts

@@ -0,0 +1,108 @@
+import { fork } from 'node:child_process';
+import { on, once } from 'node:events';
+import { readFile, unlink, writeFile } from 'node:fs/promises';
+import * as path from 'node:path';
+
+import { expect } from 'chai';
+import { parseSnapshot } from 'v8-heapsnapshot';
+
+import { MongoClient } from '../../mongodb';
+import { TestConfiguration } from '../../tools/runner/config';
+
+export type ResourceTestFunction = (options: {
+  MongoClient: typeof MongoClient;
+  uri: string;
+  iteration: number;
+}) => Promise<void>;
+
+const RESOURCE_SCRIPT_PATH = path.resolve(__dirname, '../../tools/fixtures/resource_script.in.js');
+const DRIVER_SRC_PATH = JSON.stringify(path.resolve(__dirname, '../../../lib'));
+
+export async function testScriptFactory(
+  name: string,
+  uri: string,
+  iterations: number,
+  func: ResourceTestFunction
+) {
+  let resourceScript = await readFile(RESOURCE_SCRIPT_PATH, { encoding: 'utf8' });
+
+  resourceScript = resourceScript.replace('DRIVER_SOURCE_PATH', DRIVER_SRC_PATH);
+  resourceScript = resourceScript.replace('FUNCTION_STRING', `(${func.toString()})`);
+  resourceScript = resourceScript.replace('NAME_STRING', JSON.stringify(name));
+  resourceScript = resourceScript.replace('URI_STRING', JSON.stringify(uri));
+  resourceScript = resourceScript.replace('ITERATIONS_STRING', `${iterations}`);
+
+  return resourceScript;
+}
+
+/**
+ * A helper for running arbitrary MongoDB Driver scripting code in a resource information collecting script
+ *
+ * **The provided function is run in an isolated Node.js process**
+ *
+ * A user of this function will likely need to familiarize themselves with the surrounding scripting, but briefly:
+ * - Every MongoClient you construct should have an asyncResource attached to it like so:
+ * ```js
+ * mongoClient.asyncResource = new this.async_hooks.AsyncResource('MongoClient');
+ * ```
+ * - You can perform any number of operations and connects/closes of MongoClients
+ * - The result of this function will be:
+ *   - the startup and teardown memory usage
+ *   - the number of AsyncResources with type === 'MongoClient' that did not get cleaned up by a destroy hook
+ *   - the heap snapshot parsed by 'v8-heapsnapshot'
+ *
+ * @param name - the name of the script, this defines the name of the file, it will be cleaned up if the function returns successfully
+ * @param config - `this.configuration` from your mocha config
+ * @param func - your javascript function, you can write it inline! this will stringify the function, use the references on the `this` context to get typechecking
+ * @param options - settings for the script
+ * @throws Error - if the process exits with failure
+ */
+export async function runScript(
+  name: string,
+  config: TestConfiguration,
+  func: ResourceTestFunction,
+  { iterations = 100 } = {}
+) {
+  const scriptName = `${name}.cjs`;
+  const heapsnapshotFile = `${name}.heapsnapshot.json`;
+
+  const scriptContent = await testScriptFactory(name, config.url(), iterations, func);
+  await writeFile(scriptName, scriptContent, { encoding: 'utf8' });
+
+  const processDiedController = new AbortController();
+  const script = fork(scriptName, { execArgv: ['--expose-gc'] });
+  // Interrupt our awaiting of messages if the process crashed
+  script.once('close', exitCode => {
+    if (exitCode !== 0) {
+      processDiedController.abort(new Error(`process exited with: ${exitCode}`));
+    }
+  });
+
+  const messages = on(script, 'message', { signal: processDiedController.signal });
+  const willClose = once(script, 'close');
+
+  const starting = await messages.next();
+  const ending = await messages.next();
+
+  const startingMemoryUsed = starting.value[0].startingMemoryUsed;
+  const endingMemoryUsed = ending.value[0].endingMemoryUsed;
+
+  // make sure the process ended
+  const [exitCode] = await willClose;
+  expect(exitCode, 'process should have exited with zero').to.equal(0);
+
+  const heap = await readFile(heapsnapshotFile, { encoding: 'utf8' }).then(c =>
+    parseSnapshot(JSON.parse(c))
+  );
+
+  // If any of the above throws we won't reach these unlinks that clean up the created files.
+  // This is intentional so that when debugging the file will still be present to check it for errors
+  await unlink(scriptName);
+  await unlink(heapsnapshotFile);
+
+  return {
+    startingMemoryUsed,
+    endingMemoryUsed,
+    heap
+  };
+}