Deprecating/removing runtime.SetFinalizer?

[Dmitry Vyukov]

Hi!

There are several issues with runtime.SetFinalizer:
1. We still have partially imprecise GC, so a user can't rely on
finalizers in any particular case.
2. Generally any component can contain a pointer to itself (and
absence of such pointer is usually not a part of contract), so if you
have a sufficiently complex object, it can contain a pointer to
itself, which will suddenly turn finalizers off.
3. Finalizers do not run for zero-sized objects.
4. Finalizers do not run for some as-if heap allocated objects which
promoted to globals by compiler.
5. "Tiny alloc" combined several objects into single block, this can
prevent finalizers from running for collocated objects.
6. Diagnostics are incomplete: SetFinalizer silently ignores pointers
to globals and in some cases spuriously allow pointers into middle of
heap block.
7. GC does not take into account any resource pressure (e.g. ENFILE),
so you can't rely on timely resource release by finalizers.

Some of the issue probably can be addressed. But the main point is
that finalizers seem to contradict idiomatic Go code which promotes
explicit resource management. There is a single usage of SetFinalizer
for os.File.Close; and such usages can be replaced by some pure
debugging functionality (tell me what resources I forgot to release).

So instead of putting additional effort into finalizers and making
them better, I would consider making SetFinalizer a no-op (with a
timely announce), I believe this does not contradict any documentation
(because they may not run in any particular case already). And
removing SetFinalizer in Go2 entirely.

What do you think?

[Dave Cheney]

I agree. I would support removing finalisers.

> --
>
> ---
> You received this message because you are subscribed to the Google Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[David Symonds]

I have seen it misused far too many times, so I'm in favour of deprecating it.

[Dave Cheney]

Additionally I would be supportive of a change which made finalisers a noop. There are no guarantees that finalisers run before whatever resource they are guarding is exhausted as finalisation is intimately tied to garbage collection -- a sufficiently large heap will not action finalisers at all, hence if runtime.SetFinalizer does nothing that is still complaint.

> On 2 Apr 2014, at 17:43, Dmitry Vyukov <dvy...@google.com> wrote:
>

[Kamil Kisiel]

+1 on deprecating SetFinalizer. It's one of those things some people tend to sprinkle around their code much like runtime.GOMAXPROCS. In particular I've seen it in a lot of cgo code where it's being to free memory, so I think some users have the impression they don't need to explicitly free it and can just rely on the finalizer.

[Jan Mercl]

On Wed, Apr 2, 2014 at 8:43 AM, Dmitry Vyukov <dvy...@google.com> wrote:
> So instead of putting additional effort into finalizers and making
> them better, I would consider making SetFinalizer a no-op (with a
> timely announce), I believe this does not contradict any documentation
> (because they may not run in any particular case already). And
> removing SetFinalizer in Go2 entirely.

Sounds good to me. Never used SetFinalizer exactly because it was
never guaranteed to work in the first place.

-j

[Jesse McNelis]

On Wed, Apr 2, 2014 at 5:43 PM, Dmitry Vyukov <dvy...@google.com> wrote:

So instead of putting additional effort into finalizers and making
them better, I would consider making SetFinalizer a no-op (with a
timely announce), I believe this does not contradict any documentation
(because they may not run in any particular case already). And
removing SetFinalizer in Go2 entirely.

What do you think?

I think this would break a lot of bindings to C libs.

It's a common pattern to get a Finalizer to free C allocated resources, 

there is no other way to handle this besides letting the C manual memory management requirements leak in to the Go caller.

[Taru Karttunen]

On 02.04 10:43, Dmitry Vyukov wrote:
> There are several issues with runtime.SetFinalizer:

Currently the *os.File finalizers seem to work quite ok,
and for some applications the alternative is manual reference
counting (with no support in stdlib).

Explicit runtime.SetFinalizers seem quite rare in user code,
but wouldn't it be better to panic on them than to silently
change what happens typically?

- Taru Karttunen

[Dmitry Vyukov]

Break here must be taken into quotes, because it's broken already
(even if latently). The relevant points are:

1. We still have partially imprecise GC, so a user can't rely on
finalizers in any particular case.

5. "Tiny alloc" combined several objects into single block, this can
prevent finalizers from running for collocated objects.

7. GC does not take into account any resource pressure (e.g. ENFILE),
so you can't rely on timely resource release by finalizers.

If you use C, it's reasonable that you get into C business of manual
resource management. In the end we all do this for os.File, net.Conn,
http.Request.Body, etc.

[Dave Cheney]

Seconded.

If you are using finalisers for cgo resource management today, with 1.2, your code is broken, sorry. It is broken because finalisers are not, and will never be, deterministic.

[Russ Cox]

I think removing SetFinalizer entirely is a mistake.

The os.File is going to be garbage collected whether it has a finalizer or not. I would rather have the finalizer and close the fd than be mystified about where the fd leaks are coming from (especially since there would be no associated memory leak!).

It is true that SetFinalizer is misused and overused by external programmers. We can't do much about that. But for the occasional thing like os.File, it is invaluable.

Russ

[Dmitry Vyukov]

On Wed, Apr 2, 2014 at 5:45 PM, Russ Cox <r...@golang.org> wrote:
> I think removing SetFinalizer entirely is a mistake.
>
> The os.File is going to be garbage collected whether it has a finalizer or
> not. I would rather have the finalizer and close the fd than be mystified
> about where the fd leaks are coming from (especially since there would be no
> associated memory leak!).

Yeah, but if you allocate 1GB slice before opening files, you can open
1M of them w/o GCs... and all this idea goes out of window.

Leak mystery can be solved by a pure debugging functionality. For example:

func Open(...) *File {
...
debug.OpenResource("file", f) // remembers current stack and creation time
...
}

func (f *File) Close(...) {
...
debug.CloseResource("file", f)
...
}

then you can as debug package (or via http/pprof) about resource leaks.

This is easily implementable outside of std lib today as well.

[Russ Cox]

I think it is a violation of the spirit of the Go 1 compatibility doc deprecate/disable/no-op SetFinalizer during 1.x.

Russ

[Dmitry Vyukov]

On Wed, Apr 2, 2014 at 6:05 PM, Russ Cox <r...@golang.org> wrote:
> I think it is a violation of the spirit of the Go 1 compatibility doc
> deprecate/disable/no-op SetFinalizer during 1.x.

What do you think about filing an issue for "Go2"?

[Russ Cox]

It's always fine to file issues for things we want to remember to reconsider for Go 2.

[Ian Lance Taylor]

On Wed, Apr 2, 2014 at 12:05 AM, Dmitry Vyukov <dvy...@google.com> wrote:
> On Wed, Apr 2, 2014 at 10:57 AM, Jesse McNelis <jes...@jessta.id.au> wrote:
>> On Wed, Apr 2, 2014 at 5:43 PM, Dmitry Vyukov <dvy...@google.com> wrote:
>>>
>>> So instead of putting additional effort into finalizers and making
>>> them better, I would consider making SetFinalizer a no-op (with a
>>> timely announce), I believe this does not contradict any documentation
>>> (because they may not run in any particular case already). And
>>> removing SetFinalizer in Go2 entirely.
>>>
>>> What do you think?
>>
>>
>> I think this would break a lot of bindings to C libs.
>> It's a common pattern to get a Finalizer to free C allocated resources,
>> there is no other way to handle this besides letting the C manual memory
>> management requirements leak in to the Go caller.
>
> Break here must be taken into quotes, because it's broken already
> (even if latently). The relevant points are:
> 1. We still have partially imprecise GC, so a user can't rely on
> finalizers in any particular case.
> 5. "Tiny alloc" combined several objects into single block, this can
> prevent finalizers from running for collocated objects.
> 7. GC does not take into account any resource pressure (e.g. ENFILE),
> so you can't rely on timely resource release by finalizers.

None of these points are relevant. SetFinalizer permits us to tie C
memory to Go objects. If the Go object is garbage collected, the C
memory is freed. You are describing various cases in which the Go
object is not garbage collected. That's not the issue here.

> If you use C, it's reasonable that you get into C business of manual
> resource management. In the end we all do this for os.File, net.Conn,
> http.Request.Body, etc.

I don't agree. All the reasons that we have a garbage collector for
Go apply to using SetFinalizer to tie C memory to Go objects.

I agree that are few (though not zero) reasons to use SetFinalizer for
pure Go code.

Ian

[Gustavo Niemeyer]

+1 to what Ian said. I very frequently use SetFinalizer with cgo code,
exactly in the same way that the standard library uses finalizers for
file descriptors: it's not supposed to be trusted, but it's a good
fallback when people forget to close the file explicitly.

Now, if what you're saying is that you're unable to make them work,
then that's a relevant point to talk about on itself, and a different
conversation to be held than "people should not be using them".

On Wed, Apr 2, 2014 at 2:49 PM, Ian Lance Taylor <ia...@golang.org> wrote:
> None of these points are relevant. SetFinalizer permits us to tie C
> memory to Go objects. If the Go object is garbage collected, the C
> memory is freed. You are describing various cases in which the Go
> object is not garbage collected. That's not the issue here.

(...)

> I don't agree. All the reasons that we have a garbage collector for
> Go apply to using SetFinalizer to tie C memory to Go objects.
>
> I agree that are few (though not zero) reasons to use SetFinalizer for
> pure Go code.

gustavo @ http://niemeyer.net

[Dmitry Vyukov]

FTR:
https://code.google.com/p/go/issues/detail?id=7697

[Stephen Gutekanst]

I see now that I've been largely misusing finalizers, I thought it was common practice to use them with cgo (I guess it is, just not a good practice) and had hope they would become more resilient (and.. not be removed). From what I understand here:

1. Finalizers for freeing C things is not currently guaranteed and can effectively leak resources.
2. All CGO users should (must?) switch to using manually free'd resources (ala 'defer f.Close()') 

I do not understand the details of the Go runtime implementation, but #2 above IMO will be very harmful to the Go community as a whole when CGO is involved. Complex C hierarchies often require tricky memory management (especially for C objects that must remain for some given lifetime not easily determined, e.g. OpenGL or game engines), and right now we can use finalizers as a backup-way to free C things and offer explicit free'ing on-top of finalizers. I think to new people coming to the language it will be a weird and awkward thing ("Go is garbage collected unless C wrappers to anything are involved"), I'm not arguing that it doesn't make sense practically but I think it will be a turn-away for a lot of newcomers to Go.

Stephen

[Russ Cox]

On Thu, Apr 3, 2014 at 6:42 AM, Stephen Gutekanst <stephen....@gmail.com> wrote:

I see now that I've been largely misusing finalizers, I thought it was common practice to use them with cgo (I guess it is, just not a good practice) and had hope they would become more resilient (and.. not be removed). From what I understand here:

1. Finalizers for freeing C things is not currently guaranteed and can effectively leak resources.
2. All CGO users should (must?) switch to using manually free'd resources (ala 'defer f.Close()') 

I do not understand the details of the Go runtime implementation, but #2 above IMO will be very harmful to the Go community as a whole when CGO is involved. Complex C hierarchies often require tricky memory management (especially for C objects that must remain for some given lifetime not easily determined, e.g. OpenGL or game engines), and right now we can use finalizers as a backup-way to free C things and offer explicit free'ing on-top of finalizers. I think to new people coming to the language it will be a weird and awkward thing ("Go is garbage collected unless C wrappers to anything are involved"), I'm not arguing that it doesn't make sense practically but I think it will be a turn-away for a lot of newcomers to Go.

I think Dmitriy's picture is gloomier than the reality. Yes, there are corner cases in which finalizers do not run, but most will not come up in real usage. The only remotely likely problem setting a finalizer on a complex structure that you didn't realize had a cycle. Instead, set the finalizer on a simple structure. Make sure the structure you are setting the finalizer on does not have a pointer to itself (even indirectly) and you will be fine. For example, if you defined:

type cpointer struct {

    p unsafe.Pointer

}

func (cp *cpointer) free() {

    C.free(cp.p)

    cp.p = nil

}

func newCPointer(p unsafe.Pointer) *cpointer {

    cp := &cpointer{p}

    runtime.SetFinalizer(cp, (*cpointer).free)

    return cp

}

Then in any structure that you want to hold and finalize a C pointer, you put a *cpointer field and initialize it with the result of newCPointer, and you sidestep all the possible problems Dmitriy mentioned. And you never have to call SetFinalizer by hand again. For freeing data, the finalizer is fine. If you are doing more user-visible cleanup such as removing a window from the screen you might want to use the finalizer only as a last resort, encouraging an explicit Close in the usual case.

Russ

[Stephen Gutekanst]

Russ,

I am glad to hear that. That is how I do use finalizers to interact with C code already today. I am very aware of the cyclic structures issue (I filed issue 7546 because it made me trip) but because I came from Python reference-counting land I was not oblivious to the idea of cyclic structures.

I am not concerned with the idea of removing finalizers, I only use them for managing C memory. I am however opposed to the idea of providing no means of easy garbage collection for C objects through CGO wrappers (the  "Go is garbage collected unless C wrappers to anything are involved" part of my earlier mail). But perhaps Finalizers are not the correct answer here, what if the runtime (or compiler?) exposed some means of auto-inserting scope-based hooks which could be used for reference counting?

In practice it would look something like: http://play.golang.org/p/M6_szMe_4J

Just an small thought, I haven't put too much time into it.

Stephen

[Jan Mercl]

On Thu, Apr 3, 2014 at 1:03 PM, Russ Cox <r...@golang.org> wrote:
> I think Dmitriy's picture is gloomier than the reality. Yes, there are
> corner cases in which finalizers do not run, but most will not come up in
> real usage. The only remotely likely problem setting a finalizer on a
> complex structure that you didn't realize had a cycle. Instead, set the
> finalizer on a simple structure. Make sure the structure you are setting the
> finalizer on does not have a pointer to itself (even indirectly) and you
> will be fine. For example, if you defined:
>
> type cpointer struct {
> p unsafe.Pointer
> }
>
> func (cp *cpointer) free() {
> C.free(cp.p)
> cp.p = nil
> }
>
> func newCPointer(p unsafe.Pointer) *cpointer {
> cp := &cpointer{p}
> runtime.SetFinalizer(cp, (*cpointer).free)
> return cp
> }
>
> Then in any structure that you want to hold and finalize a C pointer, you
> put a *cpointer field and initialize it with the result of newCPointer, and
> you sidestep all the possible problems Dmitriy mentioned. And you never have
> to call SetFinalizer by hand again. For freeing data, the finalizer is fine.
> If you are doing more user-visible cleanup such as removing a window from
> the screen you might want to use the finalizer only as a last resort,
> encouraging an explicit Close in the usual case.

This is all nice and well. What still makes me thinking why I should
not use it is this sentence from the documentation of SetFinalizer[0]:

"""
There is _no guarantee that finalizers will run before a program
exits_, so typically they are useful _only for releasing non-memory
resources_ associated with an object during a long-running program.
""""
(emphasizes mine)

This sentence is more or less contradicting what you wrote above, in a
certain sense. I _do_ assume that what you wrote is true and can be
actually relied upon. But then I think the documentation probably must
be changed - according to it your example is "not useful".

Preemptive nitpicking defense: gccgo and gc are not the only [future]
existing implementations of Go. The existing wording of the
SetFinalizer docs IMO allow third party implementations to
legitimately implement SetFinalizer as a nop, but that's a potential
OOM problem if used to free memory resources.

[0]: http://golang.org/pkg/runtime/#SetFinalizer

-j

[Michael Hudson-Doyle]

Jan Mercl <0xj...@gmail.com> writes:

> This is all nice and well. What still makes me thinking why I should
> not use it is this sentence from the documentation of SetFinalizer[0]:
>
> """
> There is _no guarantee that finalizers will run before a program
> exits_, so typically they are useful _only for releasing non-memory
> resources_ associated with an object during a long-running program.
> """"
> (emphasizes mine)
>
> This sentence is more or less contradicting what you wrote above, in a
> certain sense. I _do_ assume that what you wrote is true and can be
> actually relied upon. But then I think the documentation probably must
> be changed - according to it your example is "not useful".

I /think/ that when the text you quote says "memory resources" it really
means the Go heap. There is no point to using finalizers to release go
heap resources -- clearly! -- but using them to manage non go heap
memory seems to make sense. Particularly because in that case the
caveat that they might not run before program exit does not matter in
the slightest.

Cheers,
mwh

[Camilo Aguilar]

-1, I'm against removing finalizers, I do use them a lot with CGO to free C resources whose lifecycle is attached to a Go object.

[Nick Craig-Wood]

On 22/04/14 19:44, Camilo Aguilar wrote:
> -1, I'm against removing finalizers, I do use them a lot with CGO to
> free C resources whose lifecycle is attached to a Go object.

Me too, eg

https://github.com/ncw/gmp/blob/master/int.go#L80

--
Nick Craig-Wood <ni...@craig-wood.com> -- http://www.craig-wood.com/nick

[Russ Cox]

They're not going anywhere.

[Dmitry Vyukov]

I withdraw this proposal.

On Wed, Apr 23, 2014 at 9:17 PM, Russ Cox <r...@golang.org> wrote:
> They're not going anywhere.
>