This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2017年04月11日 09:33 by njs, last changed 2022年04月11日 14:58 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 1081 | closed | njs, 2017年04月11日 09:42 | |
| PR 1640 | merged | yselivanov, 2017年05月17日 23:23 | |
| Messages (8) | |||
|---|---|---|---|
| msg291469 - (view) | Author: Nathaniel Smith (njs) * (Python committer) | Date: 2017年04月11日 09:33 | |
If we have a chain of generators/coroutines that are 'yield from'ing each other, then resuming the stack works like: - call send() on the outermost generator - this enters _PyEval_EvalFrameDefault, which re-executes the YIELD_FROM opcode - which calls send() on the next generator - which enters _PyEval_EvalFrameDefault, which re-executes the YIELD_FROM opcode - ...etc. However, every time we enter _PyEval_EvalFrameDefault, the first thing we do is to check for pending signals, and if there are any then we run the signal handler. And if it raises an exception, then we immediately propagate that exception *instead* of starting to execute bytecode. This means that e.g. a SIGINT at the wrong moment can "break the chain" – it can be raised in the middle of our yield from chain, with the bottom part of the stack abandoned for the garbage collector. The fix is pretty simple: there's already a special case in _PyEval_EvalFrameEx where it skips running signal handlers if the next opcode is SETUP_FINALLY. (I don't see how this accomplishes anything useful, but that's another story.) If we extend this check to also skip running signal handlers when the next opcode is YIELD_FROM, then that closes the hole – now the exception can only be raised at the innermost stack frame. This shouldn't have any performance implications, because the opcode check happens inside the "slow path" after we've already determined that there's a pending signal or something similar for us to process; the vast majority of the time this isn't true. I'll post a PR in a few minutes that has a test case that demonstrates the problem and fails on current master, plus the fix. |
|||
| msg293887 - (view) | Author: Yury Selivanov (yselivanov) * (Python committer) | Date: 2017年05月17日 20:33 | |
New changeset ab4413a7e9bda95b6fcd517073e2a51dafaa1624 by Yury Selivanov (Nathaniel J. Smith) in branch 'master': bpo-30039: Don't run signal handlers while resuming a yield from stack (#1081) https://github.com/python/cpython/commit/ab4413a7e9bda95b6fcd517073e2a51dafaa1624 |
|||
| msg293902 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2017年05月17日 23:01 | |
The change should be backported to 3.5 and 3.6, right? The change seems very short and safe. IMHO it's ok to backport. |
|||
| msg293903 - (view) | Author: Yury Selivanov (yselivanov) * (Python committer) | Date: 2017年05月17日 23:02 | |
Yes, I'll do the backport. |
|||
| msg295578 - (view) | Author: Yury Selivanov (yselivanov) * (Python committer) | Date: 2017年06月09日 21:06 | |
New changeset e89f95bfd0881a9b80c3b1430d154a77bdf5a824 by Yury Selivanov in branch '3.6': [3.6] bpo-30039: Don't run signal handlers while resuming a yield from stack (GH-1081) (#1640) https://github.com/python/cpython/commit/e89f95bfd0881a9b80c3b1430d154a77bdf5a824 |
|||
| msg295579 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2017年06月09日 21:10 | |
Why not backporting the fix to 3.5 as well? |
|||
| msg295580 - (view) | Author: Yury Selivanov (yselivanov) * (Python committer) | Date: 2017年06月09日 21:11 | |
I don't think we need to. Isn't 3.5 is in security/important bug fix mode? I don't view this change as an important one (it's just a nice thing to have). |
|||
| msg295620 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2017年06月10日 08:52 | |
Yury Selivanov added the comment: > I don't think we need to. Isn't 3.5 is in security/important bug fix mode? I don't view this change as an important one (it's just a nice thing to have). Not yet, it still accept bug fixes: https://docs.python.org/devguide/#status-of-python-branches |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:58:45 | admin | set | github: 74225 |
| 2017年06月10日 08:52:13 | vstinner | set | messages: + msg295620 |
| 2017年06月09日 21:11:20 | yselivanov | set | messages: + msg295580 |
| 2017年06月09日 21:10:14 | vstinner | set | messages: + msg295579 |
| 2017年06月09日 21:07:32 | yselivanov | set | status: open -> closed type: behavior resolution: fixed stage: resolved |
| 2017年06月09日 21:06:42 | yselivanov | set | messages: + msg295578 |
| 2017年05月17日 23:23:43 | yselivanov | set | pull_requests: + pull_request1735 |
| 2017年05月17日 23:02:13 | yselivanov | set | messages: + msg293903 |
| 2017年05月17日 23:01:08 | vstinner | set | nosy:
+ vstinner messages: + msg293902 |
| 2017年05月17日 20:33:25 | yselivanov | set | messages: + msg293887 |
| 2017年04月24日 17:58:13 | Mariatta | set | assignee: yselivanov |
| 2017年04月11日 09:42:21 | njs | set | pull_requests: + pull_request1224 |
| 2017年04月11日 09:33:27 | njs | create | |