[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Berker Peksag]

Berker Peksag added the comment:


New changeset af88e7eda4101f36e904771d3cf59a5f740b3b00 by Berker Peksag 
(Nathaniel J. Smith) in branch 'master':
bpo-27122: Fix comment to point to correct issue number (#47)
https://github.com/python/cpython/commit/af88e7eda4101f36e904771d3cf59a5f740b3b00


--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Berker Peksag]

Berker Peksag added the comment:


New changeset 89b1824e693419b20b6a9113e5293f1c1a78065f by Berker Peksag in 
branch '3.6':
bpo-27122: Fix comment to point to correct issue number (#48)
https://github.com/python/cpython/commit/89b1824e693419b20b6a9113e5293f1c1a78065f


--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Berker Peksag]

Berker Peksag added the comment:


New changeset c6d2f498142c29ed62241ab6d89cb7b5e38f2fca by Berker Peksag in 
branch '3.5':
bpo-27122: Fix comment to point to correct issue number (#50)
https://github.com/python/cpython/commit/c6d2f498142c29ed62241ab6d89cb7b5e38f2fca


--
nosy: +berker.peksag

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Berker Peksag]

Berker Peksag added the comment:


New changeset c6d2f498142c29ed62241ab6d89cb7b5e38f2fca by GitHub in branch 
'3.5':
bpo-27122: Fix comment to point to correct issue number (#50)
https://github.com/python/cpython/commit/c6d2f498142c29ed62241ab6d89cb7b5e38f2fca


--
nosy: +berker.peksag

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Berker Peksag]

Berker Peksag added the comment:


New changeset 89b1824e693419b20b6a9113e5293f1c1a78065f by GitHub in branch 
'3.6':
bpo-27122: Fix comment to point to correct issue number (#48)
https://github.com/python/cpython/commit/89b1824e693419b20b6a9113e5293f1c1a78065f


--
nosy: +berker.peksag

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Gregory P. Smith]

Gregory P. Smith added the comment:

we talented!

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Carl Meyer]

Carl Meyer added the comment:

Greg, there was also a (different!) typo of the issue number in the code 
comment committed with this fix; that typo hasn't been fixed. Sent me on quite 
the chase looking for this bug. (I tracked down the bug independently, then 
wasn't able to repro it on trunk and found your fix and code comment). It's an 
impressive achievement to typo the same bug ID two different ways within the 
same twelve-line patch! ;-)

--
nosy: +carljm

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
status: open -> closed

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Roundup Robot]

Roundup Robot added the comment:

New changeset a14b93a4eb49 by Serhiy Storchaka in branch '3.5':
Added more tests for issue #27122.
https://hg.python.org/cpython/rev/a14b93a4eb49

New changeset ebc82b840163 by Serhiy Storchaka in branch 'default':
Added more tests for issue #27122.
https://hg.python.org/cpython/rev/ebc82b840163

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Nick Coghlan]

Nick Coghlan added the comment:

Serhiy's test changes look good to me!

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Here are additional tests.

--
status: closed -> open
Added file: http://bugs.python.org/file43447/issue27122-additional-tests.patch

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Nick Coghlan]

Nick Coghlan added the comment:

There's an existing scenario test at 
https://hg.python.org/cpython/file/default/Lib/test/test_contextlib.py#l600 and 
https://hg.python.org/cpython/file/default/Lib/test/test_contextlib.py#l648 
that aims to ensure ExitStack unwinding and context setting matches the 
behaviour of actual with statements.

It wouldn't hurt to add some explicit checks to the new test for this 
particular bug, though.

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Is it worth to add more specific tests for __context__ and __cause__ attributes 
of an exception?

--
nosy: +serhiy.storchaka

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Roundup Robot]

Roundup Robot added the comment:

New changeset 6bb68eae63e8 by Gregory P. Smith in branch '3.5':
issue27122: fix typo in the news file, wrong issue #.  not issue27123.
https://hg.python.org/cpython/rev/6bb68eae63e8

New changeset 4e1dbfcc9449 by Gregory P. Smith in branch 'default':
issue27122: fix typo in the news file, wrong issue #.  not issue27123.
https://hg.python.org/cpython/rev/4e1dbfcc9449

--
nosy: +python-dev

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Gregory P. Smith]

Changes by Gregory P. Smith :


--
resolution:  -> fixed
stage:  -> resolved
status: open -> closed
type:  -> behavior

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Gregory P. Smith]

Gregory P. Smith added the comment:

(with a follow up change to fix the typo in the news file which the bot hasn't 
seen fit to mention here yet)

i'm removing the 25782 dependency as that isn't blocking fixing this.

--
dependencies:  -CPython hangs on error __context__ set to the error itself

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Gregory P. Smith]

Gregory P. Smith added the comment:

Fixes are in 3.5 and default:

remote: notified python-check...@python.org of incoming changeset 9ee36b74b432
remote: notified python-check...@python.org of incoming changeset 9fadeee05880

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Nick Coghlan]

Changes by Nick Coghlan :


--
assignee: ncoghlan -> gregory.p.smith

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Nick Coghlan]

Nick Coghlan added the comment:

Greg, I think you should apply your change to eliminate the regression and get 
us back to the state of all stdlib context managers being well-behaved in this 
regard (we unfortunately missed the 3.5.2 release, but that will ensure it's 
fixed for 3.5.3).

Given the challenges I had trying to reproduce the hang with a plain context 
manager rather than a generator, I now the problem is specifically just #25782 
- in the absence of that, ExitStack() should misbehave in the same way actual 
nested with statements would, rather than trying to implicitly "fix" the 
misbehaving context managers.

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Gregory P. Smith]

Gregory P. Smith added the comment:

Proposed fix based on STINNER and Nick's earlier comments and the simple 
reproducer turned into a test.

Nick - your most recent comment makes me wonder if this shouldn't be doing more.

this does make both hang_bug*.py examples do something reasonable.

I'm at a loss for what Misc/NEWS entry to write for this one as I don't know 
how to accurately describe the scope of the problem.  I'd also like to avoid 
conflating issue25782's problem with this one (that causes the hang, but once 
that is fixed isn't there still a problem here to be fixed)?

--
keywords: +patch
Added file: http://bugs.python.org/file43354/issue27122-gps01.diff

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Gregory P. Smith]

Changes by Gregory P. Smith :


--
assignee: gregory.p.smith -> ncoghlan

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Gregory P. Smith]

Changes by Gregory P. Smith :


--
dependencies: +CPython hangs on error __context__ set to the error itself

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Gregory P. Smith]

Changes by Gregory P. Smith :


--
assignee:  -> gregory.p.smith

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Nick Coghlan]

Nick Coghlan added the comment:

Experimenting with this locally, it looks like the C level hang is indeed due 
to #25782, but the fact that we're implicitly trying to do "exc.__context__ = 
exc" is a new bug in _GeneratorContextManager introduced by the PEP 479 changes.

However, I'm now wondering whether that's also revealed a more general 
oversight in ExitStack's "_fix_exception_context" internal helper function: 
it's not explicitly handling the case where an __exit__ method re-raises the 
exception that was passed in, rather than returning a false value (indicating 
to the context management machinery that it should re-raise the original 
exception). I just hadn't noticed that before since all stdlib context managers 
are well-behaved in that regard (modulo bugs like the PEP 479 one).

Some failed attempts to create a simpler reproducer than Victor's last example 
show it isn't straightforward to get the current code to misbehave, but an 
upfront check for "new_exc is old_exc" may still be a worthwhile defensive 
coding measure.

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Gregory P. Smith]

Changes by Gregory P. Smith :


--
nosy: +gregory.p.smith -gps

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Emanuel Barry]

Emanuel Barry added the comment:

Yes, and it seems that it is waiting for a review. 
http://bugs.python.org/issue25782

--
nosy: +ebarry

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Yury Selivanov]

Yury Selivanov added the comment:

> 
> Maybe we should deny setting "exc.__context__=exc" (modify 
> PyException_SetContext for that). Maybe raise a new RuntimeError in this case?

There is a patch for that somewhere on the tracker

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[STINNER Victor]

STINNER Victor added the comment:

> The loop seems to be in PyErr_SetObject in a loop that recursively go through 
> PyException_GetContext.

We should also fix this function to avoid the infinite loop.

Maybe we should deny setting "exc.__context__=exc" (modify 
PyException_SetContext for that). Maybe raise a new RuntimeError in this case?

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Nick Coghlan]

Nick Coghlan added the comment:

The suggested fix looks basically correct to me - the problem is that the new 
except clause added by that patch is currently missing the "don't re-raise the 
passed in exception" logic needed to fully abide by the context management 
protocol, but we missed that on the patch review.

The only tweak I would recommend is putting that check before the check for 
StopIteration being converted to RuntimeError so it's immediately obvious to 
the reader that it takes priority over the exc.__cause__ check.

The comment on that clause should also be adjusted to reflect the fixed 
behaviour.

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[STINNER Victor]

STINNER Victor added the comment:

Simplified script to reproduce the bug.

--
nosy: +haypo
Added file: http://bugs.python.org/file42999/hang_bug2.py

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[STINNER Victor]

STINNER Victor added the comment:

Workaround, or maybe fix?, for the issue:

diff -r 0af15b8ef3b2 Lib/contextlib.py
--- a/Lib/contextlib.py Thu May 12 10:37:58 2016 +0300
+++ b/Lib/contextlib.py Wed May 25 15:56:50 2016 +0200
@@ -87,6 +87,8 @@ class _GeneratorContextManager(ContextDe
 # (see PEP 479).
 if exc.__cause__ is value:
 return False
+if exc is value:
+return
 raise
 except:
 # only re-raise if it's *not* the exception that was

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[STINNER Victor]

STINNER Victor added the comment:

It's a regression introduced by the PEP 479 in Lib/contextlib.py by the 
changeset 36a8d935c322, "PEP 479: Change StopIteration handling inside 
generators (Closes issue #22906)".

--

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[STINNER Victor]

Changes by STINNER Victor :


--
nosy: +yselivanov

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[R. David Murray]

Changes by R. David Murray :


--
nosy: +ncoghlan

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[R. David Murray]

Changes by R. David Murray :


--
nosy: +gps

___
Python tracker 

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

[issue27122] Hang with contextlib.ExitStack and subprocess.Popen (regression)

[Valentin David]

New submission from Valentin David:

The attached script hangs while using 100% on Python 3.5.1 but not on Python 
3.4.3. Tested both on Gentoo Linux and Ubuntu Linux.

The loop seems to be in PyErr_SetObject in a loop that recursively go through 
PyException_GetContext.

subprocess.Popen seems to cause the issue while calling io.open. strace reveals 
a error on call to lseek on a pipe.

The issue does not happen when using nested "with" instead of ExitStack.

--
files: hang_bug.py
messages: 266333
nosy: Valentin David
priority: normal
severity: normal
status: open
title: Hang with contextlib.ExitStack and subprocess.Popen (regression)
versions: Python 3.5
Added file: http://bugs.python.org/file42989/hang_bug.py

___
Python tracker 

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