Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-108654: restore comprehension locals before handling exception#108659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
carljm merged 6 commits intopython:mainfromcarljm:icompexc
Aug 30, 2023

Conversation

@carljm
Copy link
Member

@carljmcarljm commentedAug 30, 2023
edited by bedevere-bot
Loading

If an inlined comprehension raises an exception, insert a cleanup handler to restore any locals' values that were overwritten within the comprehension, in case an outer exception handler uses those locals.

@carljmcarljm marked this pull request as ready for reviewAugust 30, 2023 13:40
@JelleZijlstraJelleZijlstra added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelAug 30, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@JelleZijlstra for commit8e8b41f 🤖

If you want to schedule another build, you need to add the🔨 test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelAug 30, 2023
@carljm
Copy link
MemberAuthor

carljm commentedAug 30, 2023
edited
Loading

In offline conversation,@markshannon and@gvanrossum suggested that we should just always do this cleanup (regardless of whether there is an outer handler or not), for consistency of what appears in the frame under traceback inspection. The cost is only in code size (no perf cost unless an exception is raised), but overall code size is still likely smaller than the non-inlined version with a separate code object, so that's not a big concern.

I will make that update.

I'll also look into the buildbot failures here and see if any of them are legit related to this PR.

* main:pythongh-108520: Fix bad fork detection in nested multiprocessing use case (python#108568)pythongh-108590: Revertpythongh-108657 (commit400a1ce) (python#108686)pythongh-108494: Argument Clinic: Document how to generate code that uses the limited C API (python#108584)  Document Python build requirements (python#108646)pythongh-101100: Fix Sphinx warnings in the Logging Cookbook (python#108678)  Fix typo in multiprocessing docs (python#108666)pythongh-108669: unittest: Fix documentation for TestResult.collectedDurations (python#108670)pythongh-108590: Fix sqlite3.iterdump for invalid Unicode in TEXT columns (python#108657)  Revert "pythongh-103224: Use the realpath of the Python executable in `test_venv` (pythonGH-103243)" (pythonGH-108667)pythongh-106320: Remove private _Py_ForgetReference() (python#108664)  Mention Ellipsis pickling in the docs (python#103660)  Revert "Use non alternate name for Kyiv (pythonGH-108533)" (pythonGH-108649)pythongh-108278: Deprecate passing the first param of sqlite3.Connection callback APIs by keyword (python#108632)pythongh-108455: peg_generator: install two stubs packages before running mypy (python#108637)pythongh-107801: Improve the accuracy of io.IOBase.seek docs (python#108268)
@carljm
Copy link
MemberAuthor

The refleak buildbots are all failing due to a refleak intest_import that was introduced a few days ago in main:#108696

@carljm
Copy link
MemberAuthor

This buildbot has been failing with the same crash for a while now:

These Clang buildbots have all been seeing the sametest_gdb failure (seems like#104736):

PPC64 Fedora PR has been seeing the same timeout intest_math for a while.

AMD64 RHEL8 FIPS Only Blake2 Builtin Hash PR has been failingtest_socket for a while.

aarch64 Fedora Stable LTO + PGO PR andaarch64 Fedora Stable LTO PR have both been failing intest_concurrent_futures.test_shutdown multiple times already.

@carljm
Copy link
MemberAuthor

That leaves only thetest_perf_profiler failure, seen only onAMD64 Arch Linux Asan Debug PR, and not seen recently on that builder otherwise. I am not able to reproduce this failure locally in a debug+asan build, even with runningmake buildbottest in the same way the worker does. I suspect this is a flaky failure; rebuilding to see if the issue reproduces.

@carljm
Copy link
MemberAuthor

Rebuild was successful, so I believe this PR is buildbot-clean, relative to main branch.

Copy link
Member

@JelleZijlstraJelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for fixing this and thanks for chasing down all the buildbots.

carljm reacted with heart emoji
@carljmcarljm added the needs backport to 3.12only security fixes labelAug 30, 2023
@carljmcarljm merged commitd52c448 intopython:mainAug 30, 2023
@miss-islington
Copy link
Contributor

Thanks@carljm for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-108700 is a backport of this pull request to the3.12 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.12only security fixes labelAug 30, 2023
@carljmcarljm deleted the icompexc branchAugust 30, 2023 23:51
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestAug 30, 2023
…on (pythonGH-108659)(cherry picked from commitd52c448)Co-authored-by: Carl Meyer <carl@oddbird.net>Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotaarch64 Fedora Stable LTO 3.x has failed when building commitd52c448.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/336/builds/3864) and take a look at the build logs.
  4. Check if the failure is related to this commit (d52c448) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/336/builds/3864

Failed tests:

  • test.test_concurrent_futures.test_shutdown

Failed subtests:

  • test_interpreter_shutdown - test.test_concurrent_futures.test_shutdown.ProcessPoolSpawnProcessPoolShutdownTest.test_interpreter_shutdown

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

447 tests OK.

10 slowest tests:

  • test_gdb: 2 min 59 sec
  • test.test_concurrent_futures.test_wait: 1 min 11 sec
  • test.test_multiprocessing_spawn.test_processes: 1 min 10 sec
  • test_signal: 1 min 3 sec
  • test_socket: 47.5 sec
  • test.test_multiprocessing_forkserver.test_processes: 41.4 sec
  • test_io: 38.0 sec
  • test_math: 37.1 sec
  • test_subprocess: 35.5 sec
  • test.test_multiprocessing_spawn.test_misc: 33.3 sec

1 test failed:
test.test_concurrent_futures.test_shutdown

14 tests skipped:
test.test_asyncio.test_windows_events
test.test_asyncio.test_windows_utils test_devpoll test_ioctl
test_kqueue test_launcher test_startfile test_tkinter test_ttk
test_winconsoleio test_winreg test_winsound test_wmi
test_zipfile64

1 re-run test:
test.test_concurrent_futures.test_shutdown

Total duration: 3 min 24 sec

Click to see traceback logs
Traceback (most recent call last):  File"/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.lto/build/Lib/test/test_concurrent_futures/test_shutdown.py", line49, intest_interpreter_shutdownself.assertFalse(err)AssertionError: b'Exception in thread Thread-1:\nTraceback (most recent call last):\n  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.lto/build/Lib/threading.py", line 1059, in _bootstrap_inner\n    self.run()\n  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.lto/build/Lib/concurrent/futures/process.py", line 339, in run\n    self.add_call_item_to_queue()\n  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.lto/build/Lib/concurrent/futures/process.py", line 394, in add_call_item_to_queue\n    self.call_queue.put(_CallItem(work_id,\n  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.lto/build/Lib/multiprocessing/queues.py", line 94, in put\n    self._start_thread()\n  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.lto/build/Lib/multiprocessing/queues.py", line 177, in _start_thread\n    self._thread.start()\n  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.lto/build/Lib/threading.py", line 978, in start\n    _start_new_thread(self._bootstrap, ())\nRuntimeError: can\'t create new thread at interpreter shutdown\nTraceback (most recent call last):\n  File "<string>", line 1, in <module>\n  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.lto/build/Lib/multiprocessing/spawn.py", line 122, in spawn_main\n    exitcode = _main(fd, parent_sentinel)\n               ^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.lto/build/Lib/multiprocessing/spawn.py", line 132, in _main\n    self = reduction.pickle.load(from_parent)\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File "/home/buildbot/buildarea/3.x.cstratak-fedora-stable-aarch64.lto/build/Lib/multiprocessing/synchronize.py", line 115, in __setstate__\n    self._semlock = _multiprocessing.SemLock._rebuild(*state)\n                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\nFileNotFoundError: [Errno 2] No such file or directory\n' is not false

Yhg1s pushed a commit that referenced this pull requestAug 31, 2023
…ion (GH-108659) (#108700)gh-108654: restore comprehension locals before handling exception (GH-108659)(cherry picked from commitd52c448)Co-authored-by: Carl Meyer <carl@oddbird.net>Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@corona10corona10corona10 left review comments

@JelleZijlstraJelleZijlstraJelleZijlstra approved these changes

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

@iritkatrieliritkatrielAwaiting requested review from iritkatrieliritkatriel is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@carljm@bedevere-bot@miss-islington@JelleZijlstra@corona10

[8]ページ先頭

©2009-2025 Movatter.jp