[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#509803: Fails to properly free memory on exit



Sylvain Le Gall <gildor@debian.org> writes:

> Hello,
>
> On 26-12-2008, Goswin Brederlow <goswin-v-b@web.de> wrote:
>>
>> Without ocaml calling the finalizer at exit one has to manually track
>> allocated custom blocks causing 16 bytes (prev/next pointer) overhead
>> per block and implement ones own cleanup code by using atexit() in an
>> initializer, which is rather ugly.
>
> Finalizer is something purely memory related for OCaml. The problem you
> are describing is just a "hack" to avoid implementing a "close_out"-like
> function. I really think you'd better implement a "close_out" function
> for your aio binding. 
>
> Benefit of a AIO.close_out:
> - free associated resource at a precise time (memory buffer, file
>   descriptor)

At which point there can still be references from other places. So
you need extra code to detect use of a closed AIO and throw exception
in every binding using the structure.

That is the trade off between manual resource management and a GC.

> - get error status at the right time

The read/write functions have their own error handling. Since they are
asynchron you can not throw an exception or return an error to the
calling function. That one has long since run its course. The only
sane implementation is to have a closure

function Success .... -> ... | Failure ... -> ...

style interface. The _finalize itself will never return an error.

> - ensure operation is finished 
> - no need to _finalize

I still need to call a release function from libaio so the _finalize
is still needed.

If you need a .close_out function then I don't see any way around the
_finalize function as well. You will always want to call .close_out in
_finalize if it wasn't called already or you risk leaking
resources. That is just good programming.

> _finalize handle AIO.close_out:
> - error is ignored (you cannot throw exception in finalizer)

No error possible anyway here.

> - don't know when the file descriptor is freed, leading to possible
>   resource quota error (limit on memory, limit of open file descriptor)

That is always a problem with a GC.

Also look at the bigger picture. Say you use IPC shared memory. You
allocate the memory during creation time and free it in a .close
function. Now the programmer forgets that and then every time the
programm is run some shared memory will be allocated and never
freed. IPC resources don't free themself on exit.

> - don't know precisely when last operation is finished (problem when you
>   write a temporary file and then reopen it to read data back)

As long as someone has a reference to the object new operations can be
started. That is why I wanted to use the _finalize. At that point
there can't be any new operations as nothing has the object anymore.

The _finalize can never be to early (unlike a .close_out). It can only
come late.

> - need to change OCaml to make _finalizer mandatory at the end of the
>   process.

Seems like a pretty trivial change. A simple "at_exit GC.full_major"
does the trick unless a global variable is used already. Running a
full GC run after the global variables are deregistered from the root
set should not be so hard. Or rather at the very end do a full GC run
without root set. Or a trimmed down GC run that just calls _finalize
on all custom blocks.

Where is the harm?

> Honestly, I really find that there is more problem doing the way you
> propose than just making AIO.close_out mandatory just like
> Pervasives.close_out.

One of the great things of ocaml is that is so robust. You can not
have NULL pointer or references to freed memory and the ensuing
segfaults [Obj.magic excluded] or unreachable memory leaked (for
long). I don't want to introduce code that requires manual resource
tracking 

The problem with .close_out is that it can be run too early or
forgotten. Even with a close_out I would want the _finalize to give
error messages when it was forgotten.

By running the _finalize function at exit the worst that happens is
that exiting takes a while longer. As anything could randomly trigger
a full GC run at any time the code already has to cope with a GC
run at any point so no harm can come of it.


I'm not saying _finalize is the solution to all problems. But it seems
to be the only logical palce for some things. Let me give another
example:

Since with async IO the kernel does the reading/writing to memory at
an unspecified time I need a chunk of memory to read to or write from
that will not be moved by the GC. So something outside the GC heap.

So I allocate a chunk of memory and wrap that in a custom block
containing a pointer to it and its size. The _finalize function frees
the block of memory. So far so good.

Now at exit the _finalize is not called, the chunk of memory not
freed and valgrind complains. Do you believe that calls for a ".free"
binding that specifically frees the memory too? Any manual resource
tracking risks leaking the memory or freeing the memory while it is
still reachable.


>> Please forward this upstream so ocaml can be made to call the
>> finalizers at exit.
>>
>
> If you know this is an upstream bug, you should directly submit it
> upstream. This is more simple for everyone.
>
> Regards,
> Sylvain Le Gall

MfG
        Goswin


Reply to: