Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Bad interaction with tracing and calling back into Python #4609

Closed
seberg opened this issue Feb 1, 2022 · 4 comments
Closed

[BUG] Bad interaction with tracing and calling back into Python #4609

seberg opened this issue Feb 1, 2022 · 4 comments

Comments

@seberg
Copy link
Contributor

seberg commented Feb 1, 2022

Forwarding from the Pandas bug: pandas-dev/pandas#41935 (and I had also opened a Python one, but right now cython seems a more likely candidate): https://bugs.python.org/issue46451

There is an issue that calling back into Python from a Cython function can mutate module globals if the module global is a default argument (possibly not only then) and tracing is enabled and this is running on Python 3.10 (I think I confirmed for Cython 3.0 alpha as well). The repro is as easy as:

import mod

def cyfunc(arg):
    mod.function(arg)    

with mod.py:

mod_global = "something"
def function(arg, mod_global=None):
     if mod_global is None:  # probably not necessary
         mod_global = arg

I have a functioning reproducer at: https://github.com/seberg/bpo46451 importing bpo46451 will run the function and tracing will start printing None instead of the correct "this is a mod global".

I suspect this happens here: https://github.com/cython/cython/blob/master/Cython/Utility/ObjectHandling.c#L2569-L2611

@seberg
Copy link
Contributor Author

seberg commented Feb 1, 2022

To be clear, for all I know this is a problem in PyEval_EvalCodeEx. Frankly, it seems pretty likely now that I see that PyEval_EvalCodeEx is not used internally to CPython anymore and "retained as part of the API"...

@seberg
Copy link
Contributor Author

seberg commented Feb 1, 2022

Hmmm, nevermind... I think I forgot to delete the build folder and the sources were not regenerated. Cython 3 alpha seems to fix this issue, so I guess it was a known issue about frame changes in Python.

@seberg seberg closed this as completed Feb 4, 2022
@seberg
Copy link
Contributor Author

seberg commented Feb 7, 2022

We just had a report in NumPy running into this issue. Considering that this only affects debugging/profiling, I am not sure how pressing it is, though (i.e. the question is a bit if few reports currently are also because 3.10 isn't pervasive yet).

I am wondering if there is a simple solution to push? A backport of the fix, or the 3.0 release (so that numpy/pandas can do a bugfix release using that?).

@da-woods
Copy link
Contributor

da-woods commented Feb 7, 2022

The relevant commit that fixes it looks to be 4fb1e7a.

That's a fairly substantial change that I think we deliberately didn't backport. Defining the cflag CYTHON_FAST_PYCALL=0 seems to fix it though (at the cost of some speed I assume). I suspect that'd be a better solution than trying to backport to 0.29.
(We already undefine CYTHON_FAST_PYCALL on 0.29.x on Python3.11 (because other things changed))

@da-woods da-woods reopened this Feb 7, 2022
da-woods added a commit to da-woods/cython that referenced this issue Apr 14, 2022
It causes issues while profiling or debugging where global
variabels can end up inadvertently changed.

Fixes cython#4609
da-woods added a commit to da-woods/cython that referenced this issue Apr 14, 2022
It causes issues while profiling or debugging where global
variabels can end up inadvertently changed.

Fixes cython#4609
da-woods added a commit to da-woods/cython that referenced this issue Apr 14, 2022
It causes issues while profiling or debugging where global
variables can end up inadvertently changed.

Fixes cython#4609
@scoder scoder added this to the 0.29.29 milestone Apr 15, 2022
@scoder scoder closed this as completed in 5c6cf78 Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants