fix: Memory leaks on build if called more than once

Author: robertleeplummerjr

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "gpu.js",
-  "version": "2.8.4",
+  "version": "2.8.5",
   "description": "GPU Accelerated JavaScript",
   "engines": {
     "node": ">=8.0.0"

package.json

@@ -136,6 +136,7 @@ class CPUKernel extends Kernel {
    * <p>If the graphical flag is enabled, canvas is used.</p>
    */
   build() {
+    if (this.built) return;
     this.setupConstants();
     this.setupArguments(arguments);
     this.validateSettings(arguments);

src/backend/cpu/kernel.js

@@ -903,7 +903,6 @@ class Kernel {
    */
   static getSignature(kernel, argumentTypes) {
     throw new Error(`"getSignature" not implemented on ${ this.name }`);
-    return argumentTypes.length > 0 ? ':' + argumentTypes.join(',') : '';
   }
 
   /**

src/backend/kernel.js

@@ -468,6 +468,7 @@ class WebGLKernel extends GLKernel {
   }
 
   build() {
+    if (this.built) return;
     this.initExtensions();
     this.validateSettings(arguments);
     this.setupConstants(arguments);
@@ -1454,6 +1455,7 @@ float integerCorrectionModulo(float number, float divisor) {
   }
 
   destroy(removeCanvasReferences) {
+    if (!this.context) return;
     if (this.buffer) {
       this.context.deleteBuffer(this.buffer);
     }
@@ -1512,6 +1514,10 @@ float integerCorrectionModulo(float number, float divisor) {
     this.destroyExtensions();
     delete this.context;
     delete this.canvas;
+    if (!this.gpu) return;
+    const i = this.gpu.kernels.indexOf(this);
+    if (i === -1) return;
+    this.gpu.kernels.splice(i, 1);
   }
 
   destroyExtensions() {

src/backend/web-gl/kernel.js

@@ -265,18 +265,18 @@ class GPU {
      *
      * @param {IReason[]} reasons
      * @param {IArguments} args
-     * @param {Kernel} kernel
+     * @param {Kernel} _kernel
      * @returns {*}
      */
-    function onRequestSwitchKernel(reasons, args, kernel) {
-      if (kernel.debug) {
+    function onRequestSwitchKernel(reasons, args, _kernel) {
+      if (_kernel.debug) {
         console.warn('Switching kernels');
       }
       let newOutput = null;
-      if (kernel.signature && !switchableKernels[kernel.signature]) {
-        switchableKernels[kernel.signature] = kernel;
+      if (_kernel.signature && !switchableKernels[_kernel.signature]) {
+        switchableKernels[_kernel.signature] = _kernel;
       }
-      if (kernel.dynamicOutput) {
+      if (_kernel.dynamicOutput) {
         for (let i = reasons.length - 1; i >= 0; i--) {
           const reason = reasons[i];
           if (reason.type === 'outputPrecisionMismatch') {
@@ -285,42 +285,42 @@ class GPU {
         }
       }
 
-      const Constructor = kernel.constructor;
-      const argumentTypes = Constructor.getArgumentTypes(kernel, args);
-      const signature = Constructor.getSignature(kernel, argumentTypes);
+      const Constructor = _kernel.constructor;
+      const argumentTypes = Constructor.getArgumentTypes(_kernel, args);
+      const signature = Constructor.getSignature(_kernel, argumentTypes);
       const existingKernel = switchableKernels[signature];
       if (existingKernel) {
         return existingKernel;
       }
 
       const newKernel = switchableKernels[signature] = new Constructor(source, {
         argumentTypes,
-        constantTypes: kernel.constantTypes,
-        graphical: kernel.graphical,
-        loopMaxIterations: kernel.loopMaxIterations,
-        constants: kernel.constants,
-        dynamicOutput: kernel.dynamicOutput,
-        dynamicArgument: kernel.dynamicArguments,
-        context: kernel.context,
-        canvas: kernel.canvas,
-        output: newOutput || kernel.output,
-        precision: kernel.precision,
-        pipeline: kernel.pipeline,
-        immutable: kernel.immutable,
-        optimizeFloatMemory: kernel.optimizeFloatMemory,
-        fixIntegerDivisionAccuracy: kernel.fixIntegerDivisionAccuracy,
-        functions: kernel.functions,
-        nativeFunctions: kernel.nativeFunctions,
-        injectedNative: kernel.injectedNative,
-        subKernels: kernel.subKernels,
-        strictIntegers: kernel.strictIntegers,
-        debug: kernel.debug,
-        gpu: kernel.gpu,
+        constantTypes: _kernel.constantTypes,
+        graphical: _kernel.graphical,
+        loopMaxIterations: _kernel.loopMaxIterations,
+        constants: _kernel.constants,
+        dynamicOutput: _kernel.dynamicOutput,
+        dynamicArgument: _kernel.dynamicArguments,
+        context: _kernel.context,
+        canvas: _kernel.canvas,
+        output: newOutput || _kernel.output,
+        precision: _kernel.precision,
+        pipeline: _kernel.pipeline,
+        immutable: _kernel.immutable,
+        optimizeFloatMemory: _kernel.optimizeFloatMemory,
+        fixIntegerDivisionAccuracy: _kernel.fixIntegerDivisionAccuracy,
+        functions: _kernel.functions,
+        nativeFunctions: _kernel.nativeFunctions,
+        injectedNative: _kernel.injectedNative,
+        subKernels: _kernel.subKernels,
+        strictIntegers: _kernel.strictIntegers,
+        debug: _kernel.debug,
+        gpu: _kernel.gpu,
         validate,
-        returnType: kernel.returnType,
-        onIstanbulCoverageVariable: kernel.onIstanbulCoverageVariable,
-        removeIstanbulCoverage: kernel.removeIstanbulCoverage,
-        tactic: kernel.tactic,
+        returnType: _kernel.returnType,
+        onIstanbulCoverageVariable: _kernel.onIstanbulCoverageVariable,
+        removeIstanbulCoverage: _kernel.removeIstanbulCoverage,
+        tactic: _kernel.tactic,
         onRequestFallback,
         onRequestSwitchKernel,
       });
@@ -342,19 +342,20 @@ class GPU {
       onRequestSwitchKernel
     }, settingsCopy);
 
-    const kernelRun = kernelRunShortcut(new this.Kernel(source, mergedSettings));
+    const kernel = new this.Kernel(source, mergedSettings);
+    const kernelRun = kernelRunShortcut(kernel);
 
     //if canvas didn't come from this, propagate from kernel
     if (!this.canvas) {
-      this.canvas = kernelRun.canvas;
+      this.canvas = kernel.canvas;
     }
 
     //if context didn't come from this, propagate from kernel
     if (!this.context) {
-      this.context = kernelRun.context;
+      this.context = kernel.context;
     }
 
-    this.kernels.push(kernelRun);
+    this.kernels.push(kernel);
 
     return kernelRun;
   }

src/gpu.js

@@ -113,6 +113,9 @@ test('kernel.texture is deleted', (t) => {
 });
 
 function testKernelMappedTexturesAreDeleted(done) {
+  const mockGPU = {
+    kernels: []
+  };
   const webGLTexture1 = {};
   const mockTextureDelete1 = sinon.spy();
   const kernelTexture1 = {
@@ -127,22 +130,56 @@ function testKernelMappedTexturesAreDeleted(done) {
   };
   const mockContext = {};
   const mockKernelInstance = {
+    gpu: mockGPU,
     mappedTextures: [kernelTexture1, kernelTexture2],
     textureCache: [],
     context: mockContext,
     destroyExtensions: () => {},
   };
+  mockGPU.kernels.push(mockKernelInstance);
   mockKernelInstance.destroy = WebGLKernel.prototype.destroy.bind(mockKernelInstance);
   GPU.prototype.destroy.call({ kernels: [mockKernelInstance] })
     .then(() => {
       assert.equal(mockTextureDelete1.callCount, 1);
       assert.equal(mockTextureDelete2.callCount, 1);
+      assert.equal(mockGPU.kernels.length, 0);
       assert.ok(true);
       done();
+    })
+    .catch((e) => {
+      console.error(e);
     });
 }
 
 test('kernel.mappedTextures are deleted', (t) => {
   const done = t.async();
   testKernelMappedTexturesAreDeleted(done);
-});
+});
+
+test('gpu.kernels is populated and removed by kernel', () => {
+  const gpu = new GPU();
+  const kernel = gpu.createKernel(function() {
+    return 1;
+  });
+  assert.equal(gpu.kernels.length, 1);
+  assert.equal(gpu.kernels.indexOf(kernel.kernel), 0);
+  kernel.destroy();
+  assert.equal(gpu.kernels.length, 0);
+  gpu.destroy();
+});
+
+test('gpu.kernels is populated and removed by gpu', t => {
+  const done = t.async();
+  const gpu = new GPU();
+  const kernel = gpu.createKernel(function() {
+    return 1;
+  });
+  assert.equal(gpu.kernels.length, 1);
+  assert.equal(gpu.kernels.indexOf(kernel.kernel), 0);
+
+  gpu.destroy()
+    .then(() => {
+      assert.equal(gpu.kernels.length, 0);
+      done();
+    });
+});

test/features/destroy.js

@@ -0,0 +1,14 @@
+const sinon = require('sinon');
+const { assert, skip, test, module: describe, only } = require('qunit');
+const { CPUKernel } = require('../../../src');
+
+describe('internal: CPUKernel');
+
+test('.build() checks if already built, and returns early if true', () => {
+  const mockContext = {
+    built: true,
+    setupConstants: sinon.spy(),
+  };
+  CPUKernel.prototype.build.apply(mockContext);
+  assert.equal(mockContext.setupConstants.callCount, 0);
+});

test/internal/backend/cpu-kernel.js

@@ -1,3 +1,4 @@
+const sinon = require('sinon');
 const { assert, skip, test, module: describe, only } = require('qunit');
 const { WebGLKernel } = require('../../../../../src');
 
@@ -55,3 +56,12 @@ test('.validateSettings() checks output texture size - ok', () => {
   WebGLKernel.prototype.validateSettings.apply(mockContext, []);
   assert.ok(true);
 });
+
+test('.build() checks if already built, and returns early if true', () => {
+  const mockContext = {
+    built: true,
+    initExtensions: sinon.spy(),
+  };
+  WebGLKernel.prototype.build.apply(mockContext);
+  assert.equal(mockContext.initExtensions.callCount, 0);
+});