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

Misleading __reduce_cython__ error on import #3545

Closed
dvarrazzo opened this issue Apr 24, 2020 · 12 comments · Fixed by #3549
Closed

Misleading __reduce_cython__ error on import #3545

dvarrazzo opened this issue Apr 24, 2020 · 12 comments · Fixed by #3549

Comments

@dvarrazzo
Copy link
Contributor

dvarrazzo commented Apr 24, 2020

Whenever you use an import instead of a cimport compiling will work but you might be graced with an unrelated error message at import time.

Can't create easily a self-contained test: I tried and the code was failing with an expected ImportError, however you can reproduce it it with:

git clone git@github.com:psycopg/psycopg3.git mypyerr
cd mypyerr
git checkout 5647fb04dc6eb3e46e2a7bde490feea5be11f2c1
cat <<HERE | patch -p1
--- a/psycopg3/types/text.pyx
+++ b/psycopg3/types/text.pyx
@@ -1,3 +1,4 @@
+from cpython.unicode import PyUnicode_DecodeAscii
 from cpython.unicode cimport PyUnicode_DecodeUTF8
 
 cdef object load_text_binary(const char *data, size_t length, void *context):
HERE

python setup.py develop
python -c "import psycopg3._psycopg3"

Which raises the error:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "psycopg3/pq/pq_cython.pyx", line 34, in init psycopg3._psycopg3
    cdef class PGconn:
AttributeError: type object 'psycopg3._psycopg3.PGconn' has no attribute '__reduce_cython__'

I believe many help requests about __reduce_cython__ are related to this.

Tested with Python 3.6 and Cython 3.0a1.

@scoder
Copy link
Contributor

scoder commented Apr 24, 2020

I believe many help requests about __reduce_cython__ are related to this.

I'm not so sure. This ticket is about a programming error, and pretty much all cases I've seen so far seemed to involve pip installations or some kind of build setup problem instead. Specifically, whenever a module reloading attempt is involved, this error can occur.

There's this code these days:

code.putln("#if CYTHON_PEP489_MULTI_PHASE_INIT")
# Most extension modules simply can't deal with it, and Cython isn't ready either.
# See issues listed here: https://docs.python.org/3/c-api/init.html#sub-interpreter-support
code.putln("if (%s) {" % Naming.module_cname)
# Hack: enforce single initialisation.
code.putln("if (%s == %s) return 0;" % (
Naming.module_cname,
Naming.pymodinit_module_arg,
))
code.putln('PyErr_SetString(PyExc_RuntimeError,'
' "Module \'%s\' has already been imported. Re-initialisation is not supported.");' %
env.module_name.as_c_string_literal()[1:-1])
code.putln("return -1;")
code.putln("}")

So we already raise an exception on attempts to import a module twice, and prevent re-initialising the same module instance more than once. I can't say right now why these two cases don't produce a better error message (or actually work) in your case. Investigation welcome.

It could actually be that attempts to re-initialise the module come after first clearing the module dict. That would (obviously) break things, and if that's the case, then we need to find a way to actually rebuild the module dict. Or, more simply, always raise an exception on re-initialisation attempts. (Or keep track of full init-clear cycles, or something like that.)

@Timtam
Copy link

Timtam commented Apr 24, 2020

I already ran into this issue two or three times myself, but not yet with one of the Cython 3.0 alpha releases, so no comment about the module loading/reinitialization improvements yet.
I however noticed that those errors always pop up whenever I have unnoticed cyclic imports in my code. The only problem about this error is that it doesn't say anything about it. I started googling what this method reduce_cython actually is, what it does and what problems it might cause, but the actual problem was never mentioned. It took me quite a bit of time to figure it out, and now i'm much smarter in that regard, but if this error also pops up in Cython 3, I can imagine newbies getting stuck for several days with it as well.

@da-woods
Copy link
Contributor

@Timtam If you have an example of the code that caused this error then it'd be useful. The example given in this issues had too many external dependencies for me to get it to run so something a bit simpler and easier to diagnose would definitely be good (if it exists)

@Timtam
Copy link

Timtam commented Apr 24, 2020

I don't have an example right now, but I might be able to put one together if I don't forget about it and find the time to do so. I'll comment if I found something out.

@da-woods
Copy link
Contributor

da-woods commented Apr 24, 2020

Right: I have a simple reproducible 2-file example of this error:

# mod1.pyx

if globals():
    # don't know why the if-statement is needed. Otherwise I get an "undefined symbol error"
    raise ImportError

cdef class C:
    cdef int a

mod1 can be compiled with cythonize -if mod1.pyx

# tester.py
try:
    import mod1
except:
    import mod1 # have another go

Running python tester.py gives:

AttributeError: type object 'mod1.C' has no attribute '__reduce_cython__'

@scoder
Copy link
Contributor

scoder commented Apr 24, 2020

"undefined symbol error"

Could be due to dead code removal when the last statement is known to raise, i.e. when there is no success return case.

@scoder
Copy link
Contributor

scoder commented Apr 24, 2020

@da-woods hmm, I tried your example in Py3.[678] with latest master but got the expected (chained) ImportError in both cases, not the AttributeError.

@da-woods
Copy link
Contributor

da-woods commented Apr 24, 2020

Ah. I think I was testing it on the 0.29 branch. You're right - it seems OK in master. It's clearly harder to reproduce than I thought

@da-woods
Copy link
Contributor

@scoder Move the exception before the class definition and it does break in master though. I've edited the example above

scoder added a commit that referenced this issue Apr 24, 2020
…porting an extension module that was already initialised before.

Closes #3545.
@scoder scoder added this to the 3.0 milestone Apr 24, 2020
@scoder
Copy link
Contributor

scoder commented Apr 24, 2020

Let's see if #3549 resolves this.

scoder added a commit that referenced this issue Apr 25, 2020
* Work around error that "__reduce_cython__ cannot be found" when re-importing an extension module that was already initialised before.
* Exclude "reimport_failure" test in Py2 and PyPy since Py2 does not allow reimports and PyPy3 apparently does nothing for them.

Closes #3545.
scoder added a commit that referenced this issue Apr 25, 2020
…porting an extension module that was already initialised before.

Backport to 0.29.x.
Closes #3545.
scoder added a commit that referenced this issue Apr 25, 2020
…porting an extension module that was already initialised before.

Backport to 0.29.x.
Closes #3545.
@scoder scoder modified the milestones: 3.0, 0.29.17 Apr 25, 2020
@scoder
Copy link
Contributor

scoder commented Apr 25, 2020

Thanks for the report @dvarrazzo and for the stripped down test case @da-woods.

@dvarrazzo
Copy link
Contributor Author

Confirmed fixed for my use case too. Thank you for working on it even if my test wasn't easy to reproduce! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants