[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-31 Thread Xavier G. Domingo

Change by Xavier G. Domingo :


--
nosy: +xgdomingo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-29 Thread Yury Selivanov

Yury Selivanov  added the comment:

Merged; closing this issue.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-29 Thread Yury Selivanov

Yury Selivanov  added the comment:


New changeset 2a2270db9be9bdac5ffd2d50929bf905e7391a06 by Yury Selivanov in 
branch 'master':
bpo-32703: Fix coroutine resource warning in case where there's an error 
(GH-5410)
https://github.com/python/cpython/commit/2a2270db9be9bdac5ffd2d50929bf905e7391a06


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Yury Selivanov

Yury Selivanov  added the comment:

I knew about that "if", but it never fully registered to me why it's there and 
what it is protecting us from ;) So I had to debug half of ceval loop before I 
stumbled upon it again ;)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Nick Coghlan

Nick Coghlan  added the comment:

Ah, and in my REPL example, the NameError was pending when the internal result 
storage was getting set back to None.

I'm not sure I even knew the "Don't complain when an exception is pending" 
check existed, so it would have taken me a long time to find that.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

Well, I feel silly then: bpo-32605

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Yury Selivanov

Yury Selivanov  added the comment:

So the problem was that _PyGen_Finalize wasn't issuing any warnings if there's 
any error set in the current tstate.  And in Nathaniel's case, the current 
error was an AttributeError('__aexit__').

This check is weird, because right before raising the warning, we call 
PyErr_Fetch to temporarily reset the current exception if any, specifically to 
raise the warning :)

The PR just removes the check.  Unless I'm missing something this should fix 
the issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Yury Selivanov

Change by Yury Selivanov :


--
keywords: +patch
pull_requests: +5243
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Yury Selivanov

Yury Selivanov  added the comment:

Ah, we should never really get to WITH_CLEANUP_START; the exception should be 
raised in BEFORE_ASYNC_WITH

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com




[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Yury Selivanov

Yury Selivanov  added the comment:

So refactoring it into "c = open_file(); async with c" just prolongs the life 
of the coroutine so that it's GCed outside of 
WITH_CLEANUP_START/WITH_CLEANUP_FINISH block.  Something weird is going on in 
that block.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Yury Selivanov

Yury Selivanov  added the comment:

The difference between these two functions is two extra opcodes: 
STORE_FAST/LOAD_FAST before BEFORE_ASYNC_WITH.  With them we have a warning.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Yury Selivanov

Yury Selivanov  added the comment:

Changing

  async def main():
async with open_file():
  pass

to

  async def main():
c = open_file()
async with c:
  pass

also makes it print the warning :)

Also I've made a test out of this snippet and running tests in refleak mode 
shows that there's indeed no refleak here.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Nick Coghlan

Nick Coghlan  added the comment:

Looking at the ceval code, I think Yury's theory is plausible, and we may also 
be leaving the interpreter's internal stack in a dubious state. Things then get 
cleaned up if you wrap the async with in a try/except or try/finally:

==
>>> async def try_main():
... try:
... async with open_file():
... pass
... finally:
... pass
... 
>>> try_main().send(None)
sys:1: RuntimeWarning: coroutine 'open_file' was never awaited
Traceback (most recent call last):
  File "", line 1, in 
  File "", line 3, in try_main
AttributeError: __aexit__
==

Unfortunately for that theory, adding braces and "Py_DECREF(POP());" to the 
relevant error handling branch *doesn't* fix the problem.

I also found another way to provoke similar misbehaviour without async with:

==
>>> async def open_file():
... pass
... 
>>> open_file()

>>> _

>>> 1
__main__:1: RuntimeWarning: coroutine 'open_file' was never awaited
1
>>> open_file()

>>> del _
Traceback (most recent call last):
  File "", line 1, in 
NameError: name '_' is not defined
>>> _

>>> 1
1
>>> _
1
>>> 
==

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Yury Selivanov

Change by Yury Selivanov :


--
components: +Interpreter Core -asyncio
nosy: +ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Nathaniel Smith

Nathaniel Smith  added the comment:

> Yury's theory: maybe BEFORE_ASYNC_WITH's error-handling path is forgetting to 
> DECREF the object.

Nope, that doesn't seem to be it. This version prints "refcount: 2" twice, 
*and* prints a proper "was never awaited" warning:

-

import sys

async def open_file():
pass

async def main():
open_file_coro = open_file()
print("refcount:", sys.getrefcount(open_file_coro))

try:
async with open_file_coro:
pass
except:
pass

print("refcount:", sys.getrefcount(open_file_coro))

coro = main()
try:
coro.send(None)
except:
pass

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue32703] 'async with' somehow suppresses unawaited coroutine warnings

2018-01-28 Thread Nathaniel Smith

New submission from Nathaniel Smith :

Example (minimal version of https://github.com/python-trio/trio/issues/425):

-

async def open_file():
pass

async def main():
async with open_file():  # Should be 'async with await open_file()'
pass

coro = main()
coro.send(None)

-

Here we accidentally left out an 'await' on the call to 'open_file', so the 
'async with' tries to look up 'CoroutineType.__aexit__', which obviously 
doesn't exist, and the program crashes with an AttributeError("__aexit__"). Yet 
weirdly, this doesn't trigger a warning about 'open_file' being unawaited. It 
should!

Yury's theory: maybe BEFORE_ASYNC_WITH's error-handling path is forgetting to 
DECREF the object.

--
components: asyncio
messages: 311052
nosy: asvetlov, giampaolo.rodola, njs, yselivanov
priority: normal
severity: normal
status: open
title: 'async with' somehow suppresses unawaited coroutine warnings
versions: Python 3.5, Python 3.6, Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com