toArray() causes memory leaks

callumhay commented 4 years ago

What is wrong?

Using toArray() appears to cause memory leaks for immutable textures (see below).

Where does it happen?

In GPU.js, running on my Windows 7 PC, headlessgl in node.js v12.18.1.

How do we replicate the issue?

Here is the code to reproduce on my system:

const pipelineFuncSettings = {
  output: [8, 8, 8],
  pipeline: true,
  returnType: 'Array(3)',
};

const clearFuncImmutable = gpu.createKernel(function(colour) {
  return [colour[0], colour[1], colour[2]];
}, {...this.pipelineFuncSettings, immutable: true, argumentTypes: {colour: 'Array(3)'}});

let framebufferTex =  clearFuncImmutable([0,0,0]);
while (true) {
  framebufferTex.delete();
  framebufferTex = clearFuncImmutable([0,0,0]);
  framebufferTex.toArray();
}

Let that run and watch the node.exe memory climb forever. If you remove the toArray() line (i.e., framebufferTex.toArray();) then the program no longer appears to leak memory.

How important is this (1-5)?

This shouldn't leak memory... or the documentation should explain clearly why this would leak memory and how to avoid it and get a CPU buffer from your textures/framebuffers without worrying about it.

Expected behavior (i.e. solution)

This shouldn't leak memory, I should be able to loop forever - grabbing a CPU array of my framebuffer texture each time - in a stable state.

callumhay commented 4 years ago

The issue exists in GLTexture (src/backend/gl/texture/index.js) in the function call to framebuffer():

 framebuffer() {
    if (!this._framebuffer) {
      this._framebuffer = this.context.createFramebuffer();
    }
    this._framebuffer.width = this.size[0];
    this._framebuffer.height = this.size[1];
    return this._framebuffer;
  }

In every iteration of the while loop, this._framebuffer is undefined, which causes the creation of a new framebuffer.

This is called within GLTextureFloat renderRawOutput():

  renderRawOutput() {
    const gl = this.context;
    const size = this.size;
    gl.bindFramebuffer(gl.FRAMEBUFFER, this.framebuffer());
    gl.framebufferTexture2D(
      gl.FRAMEBUFFER,
      gl.COLOR_ATTACHMENT0,
      gl.TEXTURE_2D,
      this.texture,
      0
    );
    const result = new Float32Array(size[0] * size[1] * 4);
    gl.readPixels(0, 0, size[0], size[1], gl.RGBA, gl.FLOAT, result);
    return result;
  }

If you simply comment out the line gl.bindFramebuffer(gl.FRAMEBUFFER, this.framebuffer()); it no longer leaks memory and appears to work fine. This looks like it's part of a larger issue with memory management and the delete() function on textures:

  delete() {
    if (this._deleted) return;
    this._deleted = true;
    if (this.texture._refs) {
      this.texture._refs--;
      if (this.texture._refs) return;
    }
    this.context.deleteTexture(this.texture);
    if (this.texture._refs === 0 && this._framebuffer) {
      this.context.deleteFramebuffer(this._framebuffer);
      this._framebuffer = null;
    }
  }

The delete method will not properly clean up if called from user code since it will set the this._deleted flag which will cause the method to always exit early... not sure what the solve for this is.