Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
Description
Most of our quickened instructions start by asserting that tracing isn't enabled, and end by callingNOTRACE_DISPATCH(), which skips tracing checks. This is safe for many of our specializations (like those forBINARY_OP andCOMPARE_OP, which operate on known safe types), but not all of them. The problem is that any time we decref an unknown object, arbitrary finalizers could run and enable tracing.
It seems that our current practice is predicated on the idea that it's okay for tracing to start "sometime in the future", since we make no guarantees about when finalizers run. There are two issues with this:
- Trace functions may temporarily trace some frames, but not others.
- As mentioned, the interpreter loop is littered with asserts that will fail if this happens.
Here's a sort-of-minimal reproducer of what this can look like on debug builds:
Python3.11.0rc1+ (heads/3.11:bb0dab5c48,Sep62022,22:29:10) [GCC9.3.0]onlinuxType"help","copyright","credits"or"license"formoreinformation.>>>classC:...def__init__(self,x):...self.x=x...def__del__(self):...ifself.x:...breakpoint()...>>>deff(x):...C(x).x,C...>>>for _inrange(8):...f(False)...>>>f(True)--Return--><stdin>(6)__del__()->None(Pdb)nextpython:Python/ceval.c:3076:_PyEval_EvalFrameDefault:Assertion `cframe.use_tracing==0'failed.Aborted
In this caseLOAD_ATTR_INSTANCE_VALUE'sNOTRACE_DISPATCH() invalidatesLOAD_GLOBAL_MODULE'sassert(cframe.use_tracing == 0).
One option is to just rip out the asserts, which solves the more serious issue of crashing debug builds. However, I think that the best solution may be to stop usingNOTRACE_DISPATCH() altogether. It's really hard to get right, and only saves us a single memory load and bitwise|. I benchmarked a branch that usesDISPATCH() for all instructions, and the result was "1.00x slower" thanmain.@markshannon tried the same thing, and got "1.01x slower". So going this route isn't free, but also not a huge penalty for correctness and peace-of-mind.
Flagging as a release blocker, since 3.11 is affected.
Metadata
Metadata
Assignees
Labels
Projects
Status