Matt Campbell wrote:
> I noticed that when a user uploads a file to a Quixote application, the
> resulting temporary file isn't automatically removed after the request
> is processed.

....

> To prevent this problem, I think the Upload class should have a __del__
> method which deletes the file referenced by an instance's tmp_filename
> attribute if the file still exists.  That way, the application developer
> doesn't have to be concerned about cleaning up after unexpected
> uploads.  Is there anything wrong with this solution?


There are no cyclical references in an Upload instance, so __del__ should
always be called as long as the interpreter is still running. In most cases,
this should be fine.

However, this might not be guaranteed in a plain-CGI deployment: the CGI
handler (the interpreter) could die before __del__ is called (since __del__
calls are not guaranteed at interpreter shutdown). I don't do plain-CGI, so I
can't prove this is the case, but it's the one deployment model I can think of
where the interpreter shuts down rather quickly.

For completeness, I think I would vote for a .cleanup() method on the Upload
class, which could be called from __del__() but could also be called explicitly
from a try...finally block in your handler code. Something like...


class Upload:

     _cleaned_up = False

     def __del__(self):
         self.cleanup()

     def cleanup(self):
         if not self._cleaned_up:
             try:
                 if os.path.isfile(self.tmp_filename):
                     os.remove(self.tmp_filename)
             except:
                 print >> sys.stderr, \
                          'error deleting temp file %s' % self.tmp_filename
             self._cleaned_up = True


def my_handler(request):
     upload = request.form['upload']
     try:
         process(upload)
     finally:
         upload.cleanup()


I would probably write the try...finally form in my handlers anyway, just to
feel like I had done the right thing. But a cleanup() method might save me a
little thinking and a few lines of code. The _cleaned_up attribute might be
overkill but would save a little I/O and that's a good thing.

-- Graham