Skip to content

runtime: spell out rules for finalizers, and implement them #13347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ianlancetaylor opened this issue Nov 21, 2015 · 10 comments
Closed

runtime: spell out rules for finalizers, and implement them #13347

ianlancetaylor opened this issue Nov 21, 2015 · 10 comments
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Nov 21, 2015

While finalizers are problematic in various ways, they are part of the runtime package. The rules for finalizers are too vague.

For example, it is tempting to use them with C pointers. Store the C pointer as a field in a Go struct, and put a finalizer on a pointer to the Go struct. When the Go struct is finalized, free the C pointer. Unfortunately, the very last time you use the Go object, it is dead as you are making the call with the C pointer. There is technically nothing that prevents the finalizer from being called as you pass the pointer to C, so the finalizer can run and free the pointer before the C code gets a chance to execute.

One way to fix this problem would be for the compiler to force all receivers to be live until the end of a method. Then a finalizer is safe if it is a method.

@ianlancetaylor ianlancetaylor self-assigned this Nov 21, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.7 milestone Nov 21, 2015
@randall77
Copy link
Contributor

I believe the compiler already does that - input args are kept live until the end of the function, including the receiver for methods. I believe it was done for debugging/stack traces.
So this may just be a matter of committing to that implementation with a big comment in the code. +doc for the finalizers.

@cznic
Copy link
Contributor

cznic commented Nov 21, 2015

Until finalizers are guaranteed to run I'd argue that they should not be used as up to now any conforming Go implementation is free to ignore finalizers completely. On topic: Any chances to add the guarantee to the rules for the finalizers in the future?

@ianlancetaylor
Copy link
Contributor Author

@cznic What meaningful guarantee could we add? Since there is no guarantee about when a GC runs, I don't see how we could say anything about a finalizer.

@randall77 Do you think we would need to add anything special for a function that might be inlined?

@cznic
Copy link
Contributor

cznic commented Nov 21, 2015

@ianlancetaylor Perhaps something like: When the GC collects an unreachable item having a finalizer set, that finalizer is run before the item is recycled.

@dr2chase
Copy link
Contributor

@cznic You understand that's not the problem that's biting us here, right? What we're worried about is where an important field is loaded from an object (like an OS file or socket descriptor), the object goes unreachable at some point within a function, yet the field value is still in use. If the object has a finalizer, and if the finalizer (say) closes that file while the called function is still using it, that's a problem. It's neither new nor unique to either Go or native code.

This becomes more likely the more excited the optimizer is. The optimizer has to be taught some manners in the presence of finalizers.

@RLH
Copy link
Contributor

RLH commented Nov 22, 2015

Hans Boehm wrote a POPL paper on some of the problems with compiler
optimizations and finalizers. If you use finalizers you need to appreciate
some of these issues. And yes, non-deterministic asynchronous execution of
finalizers can result in subtle hard to resolve errors. As our GC
algorithms get better and our compiler optimizations get better this is
likely to become more of an issue, not less.

http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.10.4159&rep=rep1&type=pdf

On Sat, Nov 21, 2015 at 5:53 PM, dr2chase notifications@github.com wrote:

@cznic https://github.com/cznic You understand that's not the problem
that's biting us here, right? What we're worried about is where an
important field is loaded from an object (like an OS file or socket
descriptor), the object goes unreachable at some point within a function,
yet the field value is still in use. If the object has a finalizer, and if
the finalizer (say) closes that file while the called function is still
using it, that's a problem. It's neither new nor unique to either Go or
native code.

This becomes more likely the more excited the optimizer is. The optimizer
has to be taught some manners in the presence of finalizers.


Reply to this email directly or view it on GitHub
#13347 (comment).

@randall77
Copy link
Contributor

Ian, yes, inlining breaks that scheme. But, at least now, we won't inline in these situations, as there is a call into C in the code in question. We don't currently inline non-leaf methods.

@DemiMarie
Copy link
Contributor

.NET provides a GC.KeepAlive function that does exactly what it sounds like: ensures that an object reference (its argument) will stay alive until the function is called. This can be implemented in Go using an empty function that is guaranteed not to be inline (as it is in fact implemented in C#).

@ianlancetaylor
Copy link
Contributor Author

After discussion with the runtime team, we believe that the best approach is an analogue what .Net does: a runtime.KeepAlive function. That reduces the problem to one of documentation.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/23102 mentions this issue.

gopherbot pushed a commit that referenced this issue May 18, 2016
Update #13347.

Change-Id: I04bf317ed409478a859355f833d4a5e30db2b9c9
Reviewed-on: https://go-review.googlesource.com/23226
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators May 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants