ammarhakim / gkyl

This is the main source repo for the Gkeyll 2.0 code. Please see gkeyll.rtfd.io for details.
https://gkeyll.readthedocs.io/en/latest/
62 stars 18 forks source link

g0-merge/ZeroArray: __gc not called #85

Closed liangwang0734 closed 2 years ago

liangwang0734 commented 2 years ago

Description of the bug

In the g0-merge, ZeroArray.Array.__gc() is never called.

Impact

This is causing memory leak, but only at the end of a simulation when all the bulk data arrays go out of scope. In other word, it perhaps doesn't affect simulation results.

Possible causes

The cdata being wrapped, struct gkyl_array is created by an external function call gkyl_array_new, not by ffi itself through new. Therefore, no implicit ffi.gc call is automatically done during the creation of an instance. See this post.

https://github.com/ammarhakim/gkyl/blob/a2931e76f0939f6f90ff35a025a55033ffacb57d/DataStruct/ZeroArray.lua#L389

Possible solutions

local array_fn = { copy = function (self, src) return ffiC.gkyl_array_copy(self.arr, src.arr) end, ... }

local array_mt = { __new = function (self, atype, ncomp, size, on_gpu) local container = new(self) container.arr = ffiC.gkyl_array_new(atype, ncomp, size) return container end, gc = function(self) self:release() end, index = array_fn, }



@nmandell @ammarhakim @JunoRavin 
liangwang0734 commented 2 years ago

It turns out less intrusive to manually attach a finalizer according to this explanation.

See https://github.com/liangwang0734/gkyl/commit/21e530560d678950ad4e4084ff5e85da04fb35fb

This will be included in a coming PR regarding merging g0 fluid into g2. But one may cherry-pick this commit (or download the commit and sign it off) to avoid possible conflict.

New usage example:

-- old:
-- return ffiC.gkyl_array_new(atype, ncomp, size)
-- new:
return ffi.gc(ffiC.gkyl_array_new(atype, ncomp, size), ffiC.gkyl_array_release)