Fix _TracebackLogger

Fix _TracebackLogger

[GvR, 2014-02-27 18:04:24]

This whole thing looks terribly misguided. If run_until_complete() leaves
activate() in its ready queue, it is likely that we're either going to exit the
program soon, or call run_forever() soon, in which case activate() will still be
called.

The exception is the test suite; we should figure out some other way of dealing
with that (although I suppose leaks alone wouldn't be a big issue -- unreported
tracebacks might be, though).

Another possible exception might be if someone creates an event loop just to do
one small thing (e.g. one HTTP request) and then closes it again, and somehow
this gets done over and over. But that sounds like abuse of the event loop API
and not a use case we should support (at least we shouldn't make the experience
in other use cases worse to support it).

https://codereview.appspot.com/69350045/diff/1/asyncio/futures.py
File asyncio/futures.py (right):

https://codereview.appspot.com/69350045/diff/1/asyncio/futures.py#newcode107
asyncio/futures.py:107: self.activate()
This looks wrong -- if activate() hasn't been called the point is that we
*shouldn't* be dumping the traceback, because something else scheduled before
activate() runs might still call clear().

https://codereview.appspot.com/69350045/diff/1/asyncio/futures.py#newcode183
asyncio/futures.py:183: 'message': 'Future/Task exception was never retrieved:',
I don't like this. None of the other messages have a trailing comma.

https://codereview.appspot.com/69350045/diff/1/asyncio/futures.py#newcode328
asyncio/futures.py:328: # a destructore (__del__ method).
But the only reason that activate() isn't called (nor clear()) would be that we
exited the event loop before all events were run. However, that only seems a
problem with unit tests (in other cases, if we stop running the event loop we're
also going to exit the program soon, so memory leaks are not an issue).

https://codereview.appspot.com/69350045/diff/1/asyncio/test_utils.py
File asyncio/test_utils.py (right):

https://codereview.appspot.com/69350045/diff/1/asyncio/test_utils.py#newcode302
asyncio/test_utils.py:302: super().close()
This is fine.

[haypo_gmail, 2014-02-27 18:08:12]

https://codereview.appspot.com/69350045/diff/1/asyncio/futures.py
File asyncio/futures.py (right):

https://codereview.appspot.com/69350045/diff/1/asyncio/futures.py#newcode183
asyncio/futures.py:183: 'message': 'Future/Task exception was never retrieved:',
On 2014/02/27 18:04:24, GvR wrote:
> I don't like this. None of the other messages have a trailing comma.

It's to get the same output with Python 3.3 and Python 3.4, see
_TracebackLogger.__del__():
msg = 'Future/Task exception was never retrieved:\n{tb}'

[GvR, 2014-02-27 18:26:46]

https://codereview.appspot.com/69350045/diff/1/asyncio/futures.py
File asyncio/futures.py (right):

https://codereview.appspot.com/69350045/diff/1/asyncio/futures.py#newcode183
asyncio/futures.py:183: 'message': 'Future/Task exception was never retrieved:',
On 2014/02/27 18:08:12, haypo_gmail wrote:
> On 2014/02/27 18:04:24, GvR wrote:
> > I don't like this. None of the other messages have a trailing comma.
> 
> It's to get the same output with Python 3.3 and Python 3.4, see
> _TracebackLogger.__del__():
> msg = 'Future/Task exception was never retrieved:\n{tb}'

Then I'd rather change the Python 3.3 output to remove the colon.

[yselivanov, 2014-03-11 04:37:34]

https://codereview.appspot.com/69350045/diff/1/asyncio/futures.py
File asyncio/futures.py (right):

https://codereview.appspot.com/69350045/diff/1/asyncio/futures.py#newcode331
asyncio/futures.py:331: exception.__traceback__ = None
I don't like this whole business of removing tracebacks from exceptions. And
turning on debug mode just to have tracebacks is not an option either, as I want
to have the ability to extract additional information (contents of
frame.__locals__) for detailed logging even in production. 

Please, let's not do this.

-1.

[GvR, 2014-03-11 18:09:06]

I'm with Yuri. Let's not do this.

[haypo_gmail, 2014-03-11 23:20:57]

On 2014/03/11 18:09:06, GvR wrote:
> I'm with Yuri. Let's not do this.

I agree that the whole approach is wrong. Something like that should be done
instead:
https://bitbucket.org/haypo/misc/src/tip/python/suppress_locals.py
http://bugs.python.org/issue17911