[angular1] Angular 1.X Scope Based Child Grid not getting destroyed (Memory Leak)

[ananddc1989]

I have implemented the scope based master details grid to achieve the angularCompileRows: true in the child grid, this.MD will be unique ID

DetailPanelCellRenderer.prototype.destroy = function () {
                console.log($scope["detailGridOptions" + this.MD])
                if ($scope["detailGridOptions" + this.MD].api !== null)
                    $scope["detailGridOptions" + this.MD].api.destroy();
                else
                    delete $scope["detailGridOptions" + this.MD];
            };

So destroy method not happening due to api is null while collapse, But JS based sub grid have api.
This Leading the Memory Leak

Grid Objects still in memory because destroy not happen, Please provide solution !

Thanks

[naveendanda]

Hi Team,

Even I was also facing the same issue. For past few months I have been working with UI grid where I have observed same kind of memory leak. As AG grid is having api destroy, we are shifting our focus to AG Grid. But even AG has similar kind of issue when using scope based child grid.
Can someone help us out of it ? @ceolter

Thanks in advance!!

[ananddc1989]

@ceolter , @seanlandsman ,

Can you help me out of this issue any one? sub grid is working fine with scope, but unable to destroy the api, because its null in destroy method, i haven't found any sample for angular 1.X edition for master detail grid.

Thanks
Anand

[ceolter]

the grid api is set up when the grid is created, and then destroyed when the grid is destroyed. so if the api is missing after the grid is created, it means the grid is already destroyed. so you should not be looking to call destroy on the grid after this point, as it is already destroyed!!

maybe i'm not following the issue properly, however this might help:
the api is attached to the gridOptions here:
https://github.com/ceolter/ag-grid/blob/6.2.1/src/ts/gridOptionsWrapper.ts#L60
and is removed from the gridOptions here:
https://github.com/ceolter/ag-grid/blob/6.2.1/src/ts/gridOptionsWrapper.ts#L67

if the grid is getting destroyed (and the api removed) you can find out from where by putting a breakpoint into the above???

[ananddc1989]

Thank you for the quick response , actual problem isn't destroy method, The problem is the grid objects retaining the memory and its increasing whenever switching routing between two screen A,B

here : https://plnkr.co/edit/oX9OkEcxF0Guy8nAZiQu?p=preview

Profile four is

1. default load is A
2. switch to B
3. switch to A
4. switch to B

now we see the grid object of 4 and memory is keep increasing if you see in IE memory profile.

Thanks
Anand

[bhanupradeep7]

Hi
Anand is rite, I'm also facing the same problem .. How to resolve this?

[dheerajra]

Man,I too stuck with same issue..AGGrid is awesome and personally i like it.
It will be really helpful if this blocker can be resolved ASAP.

[sudhamm]

Hi,
Even my scrum team is facing this issue observed memory leak on AG grid. Can some one fix this ASAP. This is blocker for us to complete the project. Appreciate your help!

[ceolter]

Can you advise, is the grid destroy method getting called?

It sounds like everyone here is on Angular 1 right?

If you are using Angular 1 directive of ag-Grid, then the grid calls it's destroy method as follows:

    $scope.$on("$destroy", function() {
        grid.destroy();
    });

the destroy then cleans up all resources inside the grid, so there should be no memory leak.

so either a) the grid has a memory leak or b) the grid destroy method is not getting called (so something wrong with the Angular side or the config of your applications)

so next steps, either:
a) someone debug through your code, put a breakpoint into the method above, confirm if the destroy method is called or not.
b) create a plunker demonstrating this problem, i can then look. without something broken to work with, not a lot i can do.

ps - is everyone on this thread working for the same company / project?

[ananddc1989]

@ceolter : as i mention above

can you please look at this one: https://plnkr.co/edit/oX9OkEcxF0Guy8nAZiQu?p=preview

Thank you for your support

[ceolter]

i modified the plunker and added a dummy 'Saussages' object, that just creates a random array of 1000 elements. the same happens here. this suggests it isn't a grid problem, angular is not cleaning up.

here is my plunker
https://plnkr.co/edit/bq0zNCSBo7tjTZBsS7wY?p=preview

i suggest you take out ag-Grid, keep in Saussages, and raise this issue with Angular project

in addition, i looked at what is holding the reference to ag-Grid - its' Angular, see the screenshot

looks like an AngularJS problem, so closing

[bhanupradeep7]

It doesn't seem like angular problem ..The scope is retating due to some dependecy in the code.. we need to destroy that as well..

Thank you for your support

[bhanupradeep7]

@ceolter . I tried to edit the above plunker and found that the count of Saussages objects is sticking to two irrespective of the number of times i toggle between the links when the ag-grid directive is not included in the code..

where as when I include ag-grid directive the count is adding based on number of times i toggle between the two items..

It seems like the directive is storing the reference..

How to resolve this?
Thank u in advance..

[ceolter]

for me it's not, lots of saussages get created. however either way, you can see form my screenshot that it's angular that is keeping a reference to the grid.

[ananddc1989]

Is it any other solution to clean up the grid in scope destroy method ??

[ceolter]

this is a standard garbage collection question? it looks like the scope is keeping a reference to the grid, so need to remove that reference in angular?

[ananddc1989]

Saussages objects is getting cleared !!, please check the updated plunker

@ceolter https://plnkr.co/edit/mPvWltz2jTM1N4DhgFiI?p=preview

after writing destroy method and clearing the scope saussages is not retain in the memory.

[seanw]

@ceolter I've created a new plunker from your last example with the "angularCompileRows" parameter to "false" on each controller:
https://plnkr.co/edit/xC8giugk9M5K7J6J3k0K?p=preview

For me, when "angularCompileRows:false" is set for both controllers, you can switch between page A and B over and over and the count of "Saussages" never goes above two as you'd want.

If either "angularCompileRows" is set to true then a new "Saussages" object is leaked each time that page is changed to.

Assuming you can confirm this is an ag-grid issue, is there a workaround for this? I'm getting a massive memory leak from this I think and the only workaround I have right now is to rewrite the cell rendered to not use Angular compilation.

[ceolter]

Assuming you can confirm this is an ag-grid issue, is there a workaround for this?
i have no idea why it's happening. in the ag-Grid code, each row gets a scope, and it's created with:

var scope = parentScope.$new()

each scope is then destroyed with:

$scope.$destroy()

so why this is ending up with a Angular keeping a reference to the grid, i do not know.

i don't have a workaroud :(

[seanw]

@ceolter Can we reopen if this is confirmed? This is a serious showstopper for me as it causes entire Angular components to leak each time causing leaks in my app >500MB after a few minutes of use.

I tried upgrading to Angular 1.6.0-rc2 with the latest ag-grid and the problem is still there for me. I tried adding some logging to when ag-grid rows create and destroy a scope object and it seems to be calling destroy for each call to create as you'd expect. I search for memory leak bugs in other Angular projects and unfortunately Angular memory leaks look varied and tricky to pinpoint (just calling destroy isn't always enough) so I didn't see any obvious cause.

[ceolter]

sure reopened. but to be honest, we have no idea where to start, so please don't expect to get a solution.

if someone wants to offer a solution, we are happy to take it on board.

but it seems pretty isolated, so someone who is an Angular expert could help??? (i used to use Angular 1, but i'm not an expert of it's internals, i'm expert on ag-Grid so have 50% of what's needed, if someone else can advise on the angular bit)

[ceolter]

any way to fix this found?

[seanw]

@ceolter No fix for this yet I'm afraid.

[ceolter]

ok, i'll keep this open (and also move to JIRA), but we still have not clearer idea on this end why it's happening.

[ceolter]

tracked by AG-153
https://www.ag-grid.com/ag-grid-pipeline/

[DrorOzgaon]

Are there any workarounds for this, or any fix in sight?

[DrorOzgaon]

@ceolter @seanw

I found a fix for this... I'm not gonna make a pull request since it's really simple. Just do this:

RenderedRow.prototype.angular1Compile = function (element) {
	        if (this.scope) {
	            var compiledRow = this.$compile(element)(this.scope);
	            this.destroyFunctions.push(function() {
	                compiledRow.remove();
	            });
	        }
	    };

Instead of

RenderedRow.prototype.angular1Compile = function (element) {
	        if (this.scope) {
	            this.$compile(element)(this.scope);
	        }
	    };

Can you please push this in as a fix.

Enjoy :)

[seanw]

@ceolter @DrorOzgaon Amazing! But....works for me in ag-grid@7.2.2 with Angular 1.6.2 but not in ag-grid@8.0.1. Any guesses why based on finding the previous fix?

Ag-grid 8.0.1 will give this error when you destroy the grid:

angular.js:14290 Error: Failed to execute 'removeChild' on 'Node': The node to be removed is not a child of this node.
      at RenderedCell.destroy (/node_modules/ag-grid/dist/ag-grid.js:13437:30)
      at /node_modules/ag-grid/dist/ag-grid.js:12894:81
      at /node_modules/ag-grid/dist/ag-grid.js:12784:18
      at Function.Utils.iterateObject (/node_modules/ag-grid/dist/ag-grid.js:1595:14)
      at RenderedRow.forEachRenderedCell (/node_modules/ag-grid/dist/ag-grid.js:12782:24)
      at RenderedRow.destroy (/node_modules/ag-grid/dist/ag-grid.js:12894:15)
      at /node_modules/ag-grid/dist/ag-grid.js:7755:26
      at Array.forEach (native)
      at RowRenderer.removeVirtualRows (/node_modules/ag-grid/dist/ag-grid.js:7753:23)
      at RowRenderer.destroy (/node_modules/ag-grid/dist/ag-grid.js:7707:15)

The top function in the trace is this:

RenderedCell.prototype.destroy = function () {
	        _super.prototype.destroy.call(this);
	         if (this.eParentRow) {
	            this.eParentRow.removeChild(this.getGui());
	             this.eParentRow = null;
	         }
	        if (this.cellEditor && this.cellEditor.destroy) {
	            this.cellEditor.destroy();
	        }
	        if (this.cellRenderer && this.cellRenderer.destroy) {
	            this.cellRenderer.destroy();
	        }
	    };

Version 7 is missing the if (this.eParentRow) block. If I just comment out this block in version 8 it seems to fix the leak but the table renders as if the cells have been duplicated and placed on top of each other at an offset.

That's what I've tried so far but feel out of my depth understanding how ag-grid does its cleanup and what changed between versions.

[DrorOzgaon]

@seanw Unfortunately I only played around with ag-grid v5.4 and tried to find the fix for that version.

I think the problem was occurring because of double compliation (see https://docs.angularjs.org/guide/compiler#double-compilation-and-how-to-avoid-it).

Good luck!

[ananddc1989]

I prefer to go with pure native javascript base, its better working and no memory leak with destroy method
its easy to switch or reuse with any technology upgrade like angular 2 or future ++

thanks for support

[bvallambhotla]

Anyone got a fix for this. I tried destroying the scope of elements before, they get removed in the destroy functions. Still, no luck !! I had around 5 grids in my page and this seems to slow down my application.

This is what I tried:

RenderedRow.prototype.angular1Compile = function (element) {
	        if (this.scope) {
	            var compiledRow = this.$compile(element)(this.scope);
	            this.destroyFunctions.push(function() {
	                compiledelement.scope().$destroy();
	            });
	        }
	    };

[makinggoodsoftware]

Tracked by AG-1196 see our pipeline https://www.ag-grid.com/ag-grid-pipeline/

[ArunAthisamy-03]

Can some one please let me know whether we can use latest ag-grid version with Angularjs 1.X ?