bpo-29988: Only check evalbreaker after calls and on backwards egdes. #18334
Conversation
markshannon
force-pushed
the
fewer-checks-eval-breaker
branch
from
f5399c8 to
6afa580
Compare
Codecov Report
@@ Coverage Diff @@
## master #18334 +/- ##
===========================================
+ Coverage 82.11% 83.18% +1.07%
===========================================
Files 1955 1570 -385
Lines 588629 414127 -174502
Branches 44407 44406 -1
===========================================
- Hits 483351 344508 -138843
+ Misses 95633 59978 -35655
+ Partials 9645 9641 -4
Continue to review full report at Codecov.
|
|
Basically this moves the check for eval breaker to a select few points instead of being more frequent. Any idea what negative consequences this could have? Generally things like "what code pattern now prevents a ^C from raising a KeyboardInterrupt in a timely fashion" are how I anticipate this kind of change to manifest. |
There was a problem hiding this comment.
This needs test cases to make sure that it's actually solving the problem its aiming to solve.
The draft ones I wrote at the 2017 core dev sprints can be found at https://github.com/ncoghlan/cpython/pull/2/files (writing these test cases was the original reason we added per-opcode tracing support to the eval loop)
|
When you're done making the requested changes, leave the comment: |
|
Like @gpshead, this approach looks promising to me in principle - I like it a lot better than what I tried back in 2017. Removing the So if we're super lucky, the test cases from my old branch will "just work" with this new code. And if they still don't pass, well, that's important to know as well :) |
|
@ncoghlan I've not tested your branch as https://github.com/ncoghlan/cpython/pull/2/files has some unrelated changes to |
With the new airmon-ng and airodump-ng APIs with async context managers, proper properties for results and interfaces and callable objects, the "simplified" interface that exposed the Pyrcrack class is no longer needed. Usage is more comprehensive now, and cleanup afterwards should work. Altough examples seem to not wait for __aexit__ to cleanup (thus airmon-ng's created interface would prevail) upon KeyboardInterrupt. Might be caused by https://bugs.python.org/issue29988 wich relates to bpo-29988 ( python/cpython#18334 )
|
@ncoghlan any thoughts on markshannon's question above? |
|
I added f_trace_opcodes specifically to enable the injection of exceptions after an arbitrary number of opcodes. The intended usage is the way I used it in the bpo-29988 test case: inject an exception after every bytecode offset and ensure they're all handled "as expected". I didn't want to require a special build for the testing, nor rely on an undocumented feature, so we went ahead and published it, even though we didn't manage to fix the original bug report. If we knew of another way to test this kind of thing, we wouldn't have needed to add f_trace_opcodes :) |
… that __exit__ or __aexit__ is called in with statments in case of interrupt.
markshannon
force-pushed
the
fewer-checks-eval-breaker
branch
from
6afa580 to
9530e6d
Compare
|
I think this has sat around for long enough, time to merge. |
With the new airmon-ng and airodump-ng APIs with async context managers, proper properties for results and interfaces and callable objects, the "simplified" interface that exposed the Pyrcrack class is no longer needed. Usage is more comprehensive now, and cleanup afterwards should work. Altough examples seem to not wait for __aexit__ to cleanup (thus airmon-ng's created interface would prevail) upon KeyboardInterrupt. Might be caused by https://bugs.python.org/issue29988 wich relates to bpo-29988 ( python/cpython#18334 )
Makes sure that
__exit__or__aexit__is called in (async) with statements, by not handling interrupts during set up of the with block.We want to make sure that interrupts are always handled eventually, and ideally that they are handled promptly.
Checking
eval_breakeron backward edges ensures that they are always handled eventually.Checking after every explicit call ensures that they are handled promptly in most cases.
https://bugs.python.org/issue29988