Set of changes to try to fix pending issues PR #27

[amartinhuertas]

WORK IN PROGRESS ... DO NOT MERGE !!!

@JordiManyer

I have been working on the issues which, in my view, are still pending from PR #27. I have done the changes in a separate branch/PR instead of pushing directly. Why? Among others, I am not still 100% sure if we will be able to let all this work as we intend (there are still issues to be solved in this PR).

Let me share with you what I've learned/done while reviewing PR #27 and developing/debugging code myself:

• The SC library, on which p4est relies to do memory allocation, has its own internal memory tracker. If there is still memory to be freed (allocated within p4est using mechanisms of the SC library) when calling sc_finalize(), this function generates an exception with error messages looking as:

[1,0]<stdout>:[libsc 0] Abort: Memory balance (p4est)
[1,0]<stdout>:[libsc 0] Abort: src/sc.c:704

• Due to this, I guess that the main reason you were having issues with sc_finalize() in PR #27 has to do with improper automatic deallocation of p4est objects.

• A mejor source of concern is p4est_connectivity_t. We must deallocate it properly if we want to avoid sc_finalize() generating the execption. At present, if am not wrong, PR #27 does not actually handle this situation properly. I have implemented a mechanism based on two extra member variables of OctreeDistributedModel in order to handle the automatic deallocation of p4est_connectivity_t.

• However, I am stuck because, at present, (a very narrowed down with just a couple of calls to octree models constructors) OctreeDistributedModelTests.jl fails due to "memory balance" and i am not able to see why (see https://github.com/gridap/GridapP4est.jl/actions/runs/3928266265/jobs/6715692901#step:10:32 for more details). When I execute the tests with only either of these lines the error disappears!!!

• As a general comment, it concerns me that using a closure for the f() function passed to with may prevent this mechanism to work in general. I think this is not the case for the particular case of OctreeDistributedModelTests.jl, but I can think of scenarios where this may impact us.

• Another source of concern: https://github.com/gridap/GridapP4est.jl/commit/30a1111bb8ef850dea8749895c0f83da15ae3035#diff-cd498b7d7a8e9e6a025d0292efa3c176454e98fee8b550bae973aea59bffc3eeR104

[JordiManyer]

Due to this, I guess that the main reason you were having issues with sc_finalize() in PR Automated GC #27 has to do with improper automatic deallocation of p4est objects.

Yes, this was the issue I had. I was also wondering why we were not deallocating the p4est_connectivity structure manually.

[amartinhuertas]

@JordiManyer ... PR ready to review. Please accept if you agree with the changes here (once the tests pass)

[amartinhuertas]

However, I am stuck because, at present, (a very narrowed down with just a couple of calls to octree models constructors) OctreeDistributedModelTests.jl fails due to "memory balance" and i am not able to see why (see https://github.com/gridap/GridapP4est.jl/actions/runs/3928266265/jobs/6715692901#step:10:32 for more details). When I execute the tests with only either of these lines the error disappears!!!

For the records, we reached the conclusion that the cause of this behaviour has to do with the asynchronousness of the garbage collector. There is not guarantee that all objects have been GCed when one calls sc_finalize.

[JordiManyer]

All OK from my side. Merging PR.