diff -r c1a7ba57b4ff Lib/asyncio/futures.py --- a/Lib/asyncio/futures.py Thu Dec 12 23:07:40 2013 +0100 +++ b/Lib/asyncio/futures.py Thu Dec 12 23:57:36 2013 +0100 @@ -30,79 +30,6 @@ class InvalidStateError(Error): # TODO: Show the future, its state, the method, and the required state. -class _TracebackLogger: - """Helper to log a traceback upon destruction if not cleared. - - This solves a nasty problem with Futures and Tasks that have an - exception set: if nobody asks for the exception, the exception is - never logged. This violates the Zen of Python: 'Errors should - never pass silently. Unless explicitly silenced.' - - However, we don't want to log the exception as soon as - set_exception() is called: if the calling code is written - properly, it will get the exception and handle it properly. But - we *do* want to log it if result() or exception() was never called - -- otherwise developers waste a lot of time wondering why their - buggy code fails silently. - - An earlier attempt added a __del__() method to the Future class - itself, but this backfired because the presence of __del__() - prevents garbage collection from breaking cycles. A way out of - this catch-22 is to avoid having a __del__() method on the Future - class itself, but instead to have a reference to a helper object - with a __del__() method that logs the traceback, where we ensure - that the helper object doesn't participate in cycles, and only the - Future has a reference to it. - - The helper object is added when set_exception() is called. When - the Future is collected, and the helper is present, the helper - object is also collected, and its __del__() method will log the - traceback. When the Future's result() or exception() method is - called (and a helper object is present), it removes the the helper - object, after calling its clear() method to prevent it from - logging. - - One downside is that we do a fair amount of work to extract the - traceback from the exception, even when it is never logged. It - would seem cheaper to just store the exception object, but that - references the traceback, which references stack frames, which may - reference the Future, which references the _TracebackLogger, and - then the _TracebackLogger would be included in a cycle, which is - what we're trying to avoid! As an optimization, we don't - immediately format the exception; we only do the work when - activate() is called, which call is delayed until after all the - Future's callbacks have run. Since usually a Future has at least - one callback (typically set by 'yield from') and usually that - callback extracts the callback, thereby removing the need to - format the exception. - - PS. I don't claim credit for this solution. I first heard of it - in a discussion about closing files when they are collected. - """ - - __slots__ = ['exc', 'tb'] - - def __init__(self, exc): - self.exc = exc - self.tb = None - - def activate(self): - exc = self.exc - if exc is not None: - self.exc = None - self.tb = traceback.format_exception(exc.__class__, exc, - exc.__traceback__) - - def clear(self): - self.exc = None - self.tb = None - - def __del__(self): - if self.tb: - logger.error('Future/Task exception was never retrieved:\n%s', - ''.join(self.tb)) - - class Future: """This class is *almost* compatible with concurrent.futures.Future. @@ -128,7 +55,7 @@ class Future: _blocking = False # proper use of future (yield vs yield from) - _tb_logger = None + _traceback = None def __init__(self, *, loop=None): """Initialize the future. @@ -162,6 +89,12 @@ class Future: res += '<{}>'.format(self._state) return res + def __del__(self): + if self._traceback is not None: + logger.error('Future/Task exception was never retrieved:\n%s', + ''.join(self._traceback)) + self._traceback = None + def cancel(self): """Cancel the future and schedule callbacks. @@ -214,9 +147,7 @@ class Future: raise CancelledError if self._state != _FINISHED: raise InvalidStateError('Result is not ready.') - if self._tb_logger is not None: - self._tb_logger.clear() - self._tb_logger = None + self._traceback = None if self._exception is not None: raise self._exception return self._result @@ -233,9 +164,7 @@ class Future: raise CancelledError if self._state != _FINISHED: raise InvalidStateError('Exception is not set.') - if self._tb_logger is not None: - self._tb_logger.clear() - self._tb_logger = None + self._traceback = None return self._exception def add_done_callback(self, fn): @@ -286,12 +215,11 @@ class Future: if self._state != _PENDING: raise InvalidStateError('{}: {!r}'.format(self._state, self)) self._exception = exception - self._tb_logger = _TracebackLogger(exception) self._state = _FINISHED self._schedule_callbacks() - # Arrange for the logger to be activated after all callbacks - # have had a chance to call result() or exception(). - self._loop.call_soon(self._tb_logger.activate) + self._traceback = traceback.format_exception(exception.__class__, + exception, + exception.__traceback__) # Truly internal methods.