Re: Reference-counting and finalizers that can fail are uneasy partners (was: [PATCH 0/4] qemu-img: Fix exit code for errors closing the image)

[Daniel P . Berrangé]

On Wed, Feb 22, 2023 at 01:08:05PM +0100, Markus Armbruster wrote:
> A half-baked thought has been sloshing around in my head.  Perhaps I can
> bake it some more by writing it up.
> 
> Reference-counting and finalizers that can fail are uneasy partners.
> 
> When managing lifetimes manually, you control where finalization
> happens.  When finalization can fail, you're as empowered as you could
> be to make it fail in a place where you can handle the failure sensibly.
> 
> Manual resource management is tedious and error prone, and that's a
> serious problem.  Garbage collection takes it off your hands.  Good.
> But now finalization happens at some future time, and in garbage
> collection context.  Fine when finalization's side effects are
> sufficiently harmless.  But what if finalization can fail?  We trade one
> serious problem (manual resource management) for another one (handling
> finalization failures).
> 
> Reference counting is slightly different.  Here, finalization can only
> happen at unref, which means you retain more control than with garbage
> collection.  However, we do need unrefs in places where we can't
> sensibly handle failure.  For instance, when code operates on an object
> whose reference count can be dropped concurrently, we need to guard with
> a ref/unref bracket to keep the object alive while the code is messing
> with it.
> 
> The only way out I can see is to systematically avoid finalizers that
> can fail, by extracting the part that can fail into a shutdown method,
> to be called in a suitable context, and before finalization.

Yes, I concur with pretty much everything you say above.

Since finalizers can occur in any context the logic that runs in
them needs to be quite clearly defined and free of side effects
or unpredictable failure scenarios.

I would probably go further and say that finalizers need to be
able to execute in finite time, so that callers do not have
execution of their thread blocked arbitrarily if they happen to
be one the one that releases the last reference.

Finalizers should be releasing resources that are already in
a "safe" state. Releasing memory, decrementing references,
unregistering callbacks, are typical safe things.

Performing I/O is a clearly a bad idea / inappropriate for
a finalizer by this standard.

Garbage collection vs reference counts is tangential to this
problem, as you say, they'll both share the same problem we're
facing.

> Yes, this takes us back to manual resource management, only we manage
> shutdown instead of death.
> 
> Finalizing something that has not been shut down would be a programming
> error.  A recoverable one, I guess; we can have finalize attempt to shut
> down then, and if it fails, just weep into the logs and move on.

This is approximately what I did with QIOChannel.  There is a
qio_channel_close() method that is best practice to invoke to
release resources assoociated with the channel, possibly flushing
pending I/O, and reporting failures via an Error **errp.

If this is not called, however, the finalizer will call close
on your behalf, discarding errors. It'll probably be OK much of
the time, and if we find it isn't, then the missing explicit
close call needs to be addressed.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|