python: closures and classes

Question

I need to register an atexit function for use with a class (see Foo below for an example) that, unfortunately, I have no direct way of cleaning up via a method call: other code, that I don't have control over, calls Foo.start() and Foo.end() but sometimes doesn't call Foo.end() if it encounters an error, so I need to clean up myself.

I could use some advice on closures in this context:

class Foo:
  def cleanup(self):
     # do something here
  def start(self):
     def do_cleanup():
        self.cleanup()
     atexit.register(do_cleanup)
  def end(self):
     # cleanup is no longer necessary... how do we unregister?

• Will the closure work properly, e.g. in do_cleanup, is the value of self bound correctly?

• How can I unregister an atexit() routine?

• Is there a better way to do this?

edit: this is Python 2.6.5

Therrien

I hope that's Python 3, otherwise not inheriting from object gives you one of those dreaded old-style classes... – Thermostat

old-style? [scratches head] please elaborate so I can avoid errors – Therrien

old style classes are classes from before the time classes and types were unified, if you don't inherit from object, you get an old style class. – Albuminuria

So I should be declaring class Foo(object): ? – Therrien

Yes. See python.org/download/releases/2.2.3/descrintro. – Rahel

@Glenn: :shrug: I don't have an answer that works better, and in my particular case I have only one or two instances, so technically I don't need to unregister. – Therrien

Who's going to waste time giving a better answer when a poor one is accepted so quickly, without even waiting a few hours for comments? – Deceitful

@Glenn: if you feel like there's something wrong here, then post your own improved answer, instead of throwing out nonconstructive criticism. – Therrien

Answer 1

Make a registry a global registry and a function that calls a function in it, and remove them from there when necessary.

cleaners = set()

def _call_cleaners():
    for cleaner in list(cleaners):
        cleaner()

atexit.register(_call_cleaners)

class Foo(object):
  def cleanup(self):
     if self.cleaned:
         raise RuntimeError("ALREADY CLEANED")
     self.cleaned = True
  def start(self):
     self.cleaned = False
     cleaners.add(self.cleanup)
  def end(self):
     self.cleanup()
     cleaners.remove(self.cleanup)

Poche

That doesn't work: you'll get "Set changed size during iteration" if you try it. You just need to change the for loop into for cleaner in list(cleaners) and then it should be fine. – Aaronson

Answer 2

I think the code is fine. There's no way to unregister, but you can set a boolean flag that would disable cleanup:

    class Foo:
      def __init__(self):
         self.need_cleanup = True
      def cleanup(self):
         # do something here
         print 'clean up'
      def start(self):
         def do_cleanup():
            if self.need_cleanup:
               self.cleanup()
         atexit.register(do_cleanup)
      def end(self):
         # cleanup is no longer necessary... how do we unregister?
         self.need_cleanup = False

Lastly, bear in mind that atexit handlers don't get called if "the program is killed by a signal not handled by Python, when a Python fatal internal error is detected, or when os._exit() is called."

Overprint

+1 The closure itself is pretty useless though, as you can just do atexit.register(self.cleanup) directly. – Dyslogistic

-1. This will leak entries in the atexit table; if a million of these objects are created, a million items will be left behind in the atexit table permanently. That's not fine. – Deceitful

This will also leak references to the object itself, which is also definitely not fine. – Deceitful

Answer 3

self is bound correctly inside the callback to do_cleanup, but in fact if all you are doing is calling the method you might as well use the bound method directly.

You use atexit.unregister() to remove the callback, but there is a catch here as you must unregister the same function that you registered and since you used a nested function that means you have to store a reference to that function. If you follow my suggestion of using a bound method then you still have to save a reference to it:

class Foo:
  def cleanup(self):
     # do something here
  def start(self):
     self._cleanup = self.cleanup # Need to save the bound method for unregister
     atexit.register(self._cleanup)
  def end(self):
     atexit.unregister(self._cleanup)

Note that it is still possible for your code to exit without calling ther atexit registered functions, for example if the process is aborted with ctrl+break on windows or killed with SIGABRT on linux.

Also as another answer suggests you could just use __del__ but that can be problematic for cleanup while a program is exiting as it may not be called until after other globals it needs to access have been deleted.

Edited to note that when I wrote this answer the question didn't specify Python 2.x. Oh well, I'll leave the answer here anyway in case it helps anyone else.

Aaronson

Notice that atexit.unregister only exists in Python 3. – Blastomere

Answer 4

Since shanked deleted his posting, I'll speak in favor of __del__ again:

import atexit, weakref
class Handler:
    def __init__(self, obj):
        self.obj = weakref.ref(obj)
    def cleanup(self):
        if self.obj is not None:
            obj = self.obj()
            if obj is not None:
                obj.cleanup()

class Foo:
    def __init__(self):
        self.start()

    def cleanup(self):
        print "cleanup"
        self.cleanup_handler = None

    def start(self):
        self.cleanup_handler = Handler(self)
        atexit.register(self.cleanup_handler.cleanup)

    def end(self):
        if self.cleanup_handler is None:
            return
        self.cleanup_handler.obj = None
        self.cleanup()

    def __del__(self):
        self.end()

a1=Foo()
a1.end()
a1=Foo()
a2=Foo()
del a2
a3=Foo()
a3.m=a3

This supports the following cases:

• objects where .end is called regularly; cleanup right away
• objects that are released without .end being called; cleanup when the last reference goes away
• objects living in cycles; cleanup atexit
• objects that are kept alive; cleanup atexit

Notice that it is important that the cleanup handler holds a weak reference to the object, as it would otherwise keep the object alive.

Edit: Cycles involving Foo will not be garbage-collected, since Foo implements __del__. To allow for the cycle being deleted at garbage collection time, the cleanup must be taken out of the cycle.

class Cleanup:
    cleaned = False
    def cleanup(self):
        if self.cleaned:
            return
        print "cleanup"
        self.cleaned = True
    def __del__(self):
        self.cleanup()

class Foo:
    def __init__(self):...
    def start(self):
        self.cleaner = Cleanup()
        atexit.register(Handler(self).cleanup)
    def cleanup(self):
        self.cleaner.cleanup()
    def end(self):
        self.cleanup()

It's important that the Cleanup object has no references back to Foo.

Dogwood

It will also keep the objects alive in case there's a cyclic reference anywhere involving it. If you are going to use __del__ at least make that Handler object have the __del__ method and not Foo, and store the weak reference to the Foo object, making the Handler object remove itself when the original object gets deleted. – Poche

@Rosh Oxymoron: no, having Handler implement __del__ is no good. This objects stays until exit, since it is an atexit handler. You are right that the object won't be garbage collected when in a cycle. To support this, you need another object that does the cleanup, see my edit. – Blastomere

Answer 5

Why don't you try it? It only took me a minute to check.

(Answer: Yes)

However, you can simplify it. The closure isn't needed.

class Foo:
   def cleanup(self):
      pass
   def start(self):
      atexit.register(self.cleanup)

And to not cleanup twice, just check in the cleanup method if a cleanup is needed or not before you clean up.

Intermittent

This is neat. I chose to register my cleanup call in my class init – works too. – Vernalize