[New-bugs-announce] [issue8034] logging: Improve error handing in RotatingFileHandler.doRollover()

[STINNER Victor]

New submission from STINNER Victor <victor.stinner at haypocalc.com>:

My server is running as root and is writing logs into /var/log/nucentral.log. I tried to run it under a different user ("nucentral"), and I changed /var/log/nucentral.log file permissions.

I'm using:
   MAX_LOG_FILESIZE = 5 * 1024 * 1024
   MAX_LOG_FILES = 10   # including .log, so last file prefix is .log.9

   handler = RotatingFileHandler('/var/log/nucentral.log', 'a',
      maxBytes=MAX_LOG_FILESIZE, backupCount=(MAX_LOG_FILES - 1),
      encoding='utf8')

The problems start at the next rollover: the user is not allowed to write into /var/log, and so rename /var/log/nucentral.log to /var/log/nucentral.log.1 (and /var/log/nucentral.log.2 to /var/log/nucentral.log.3) fails. We have here a new problem: doRollover() starts by closing the log file and then rename files. If rename fails, the stream is closed and no new stream is opened. But my server catchs any exception and write them into /var/log/nucentral.log. The rename exception will be written... in the log file, but writing in a closed file raise a new exception! The log system is completly broken and go into an evil recursion loop :-)

I can fix my code responsible to log any exception to avoid the recursion, and avoid the directory permission by writing into /var/log/nucentral/. But there is still the problem of closing the stream before renaming the log files.

I propose to close the stream *after* renaming old log files. Attached patch closes the stream just before reopening it. It works correctly on Linux.

I don't know if it's possible on Windows to rename a file which is currently open :-/ Anyway, the goal is to ensure that the stream is open when exiting the doRollover() function. Another idea would be to use the following pattern:
  stream.close()
  try:
     ...
  except:
     self.stream = self._open()
     raise
  else:
     self._mode = 'w'
     self.stream = self._open()

----------
components: Library (Lib)
files: logging_rotate.patch
keywords: patch
messages: 100252
nosy: haypo
severity: normal
status: open
title: logging: Improve error handing in RotatingFileHandler.doRollover()
versions: Python 2.7, Python 3.2, Python 3.3
Added file: http://bugs.python.org/file16406/logging_rotate.patch

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue8034>
_______________________________________