Sphinx 7.2.5 breaks with PyO3: "modules may only be initialized once per interpreter process" #11662
Comments
|
@pbybee Please provide a full reproducer. PyO3 is for rust projects, which you haven't mentioned in your description. Does the error appear with Sphinx 7.2.4? A |
AA-Turner
changed the title
|
No 7.2.4 builds correctly. We are not using PyO3 in our project. Perhaps it's a dependancy of sphinx that revved. I'll dig a little more as I don't have good repro steps at the moment without digging through the sphinx build process. |
|
If you're able to bisect, try before/after this commit: 8248be3 |
Very unlikely, more likely is that a dependency of the object you're documenting uses PyO3. |
|
Can you check if the bad module contains a TYPE_CHECKING block and if any of the imports in this block contains some PyO3 object somewhere (or depends on it)? Could you also give us the dependencies of your project? As Adam pointed out in another issue where the feature of reloading a module with TYPE_CHECKING enabled makes jax fails, can you check the bad module with @AA-Turner I am wondering, but for projects that would fail like that, could we perhaps make the TYPE_CHECKING feature optional? (it's probably impossible to make it optional per module due to dependencies that we would then need to build in advance). |
|
Ok I know what happened.
Reason: PyO3 seems to import modules but does not seem to "unimport" them if they are deleted from EDIT: I concluded as such by creating a module using PyO3, importing it, deleting it from. |
|
This may not apply exclusively to PyO3. We are seeing a similar issue with importing the onnx package (which is built using Pybind11). This error is not present in 7.2.4. |
|
Actually, it's probably something related to how extension modules are imported. It appears that deleting them from sys.modules is not sufficient. By the way, what is the full traceback error? you are missing the exception message (or there is none?) |
|
I've also encountered this issue in jaraco/skeleton#88 and I've produced a minimal reproducer in https://github.com/jaraco/jaraco.mongodb/tree/triage/36-minimal. Run with In this case, the offending import is |
|
Mmh thank you. As I only have my phone for investing this, it will be hard. In the meantime, maybe the issue could be directly related to PyO3? Or, alternatively, for extension modules, maybe we could skip their removal, though they would still be leftovers modules... So I don't really know what to do (except pinning the version prior to 7.1.x. I wouldn't recommend 7.2.x as this is kind of subject to some unstability). |
Correction - after ensuring that I'm forcing the reinstall, I have confirmed that commit is implicated: |
|
cc @godlygeek as the author of #11645 if you've any ideas re PyO3/extension module unloading in general. |
|
Another simple reproducer: import importlib, sys
orig_modules = frozenset(sys.modules)
module = importlib.import_module('pydantic')
for m in [m for m in sys.modules if m not in orig_modules]:
sys.modules.pop(m)
module = importlib.import_module('pydantic')
# ImportError: PyO3 modules may only be initialized once per interpreter process |
|
cc: @davidhewitt -- sorry for the ping, but I wondered if you'd have any advice here? The problem is Sphinx tries to import modules twice (with Is there an officially blessed way to unload a PyO3 module such that we're able to attempt importing it again? (Other ideas to work around this are very welcome also, I've not had much luck finding discussion on this point). Thanks, |
|
One idea I have is to allow people to just disable the new feature (it will be quite hard to disable it per a module basis since they are dependencies to know in advance, so it's better to allow turning it off completely only). |
|
Thanks for the ping, and sorry this is causing folks pain. This check was introduced as a defense against subinterpreters, which are a very complex thing to support at the native level (sharing arbitrary python objects between subinterpreters is disallowed, but "static" variables at the native level can make it dangerously easy to do so accidentally). Unfortunately the defense as implemented is hitting this edge case when the PyO3 module is removed from I'm keen to find ways to resolve this, we also hit the same issue in pydantic/pydantic#6584 Rereading the original suggestion in PyO3/pyo3#2346 (comment) to introduce this check, there was a more sophisticated form of the check:
I'll investigate if this is possible to rework the check to do this. Unfortunately it cannot fix PyO3 modules already shipped, but we can release a patch fix asap to get this through the ecosystem. |
|
For existing modules if you need a workaround for right now - the only options I can think of right now are:
... in hindsight I wish I'd implemented an environment variable |
|
Thanks for the very fast response David! To be cheeky, could I ask if there is there a 'blessed' way to introspect if a module is PyO3 generated? It seems the variables I think for now I might try and skip unloading all extension modules with A |
Well... The other approach I proposed in #11642 (comment) wouldn't have this particular issue, as far as I can see, since importlib.reload(sys.modules["pydantic_core._pydantic_core"])doesn't raise an exception. We could change this code to something like: module = import_module(modname, warningiserror=warningiserror)
try:
typing.TYPE_CHECKING = True
with contextlib.suppress(ImportError):
module = importlib.reload(module)
finally:
typing.TYPE_CHECKING = Falseand it wouldn't have this particular problem, as far as I can see. But I can't be sure it wouldn't have some other obscure problem... Ultimately, I don't think that Sphinx ought to be setting |
|
As I mentioned, reloading a module might have side-effects (like unpatching things that were set up by extensions I think) so it's maybe not a good idea. +1 for the option to enable the feature though. Ideally we should have an approach based on static analysis, extending the ModuleAnalyzer but it's a bit hard (I have something like that in my projects but it doesn't work well for some cases...) |
|
@AA-Turner great question - not something we have directly attempted to support - see PyO3/pyo3#3426, we should definitely think about adding this. |
If we only reload a module that we just imported (that is, we check that it wasn't in |
|
Err I think it's ok? To clarify here is what I understood:
Sorry I am a bit too tired now to think of more on this issue so I'll just think about it tomorrow with a reposed mind. |
|
Yes, this is what I mean. I think it ought to be... Well, reasonably safe. I think importing the module twice would actually solve most circular imports. And I think it's probably fine-ish, since we don't want to actually call any functions from the module. But there may be edge cases I'm not seeing... |
I'm seeing the "PyO3 modules may only be initialized once per interpreter process" errors when creating sphinx documentation. That may be related to more recent sphinx versions, re: sphinx-doc/sphinx#11662 Downgrade sphinx to an earlier version
|
For what it's worth, here's another example of opaque breakage brought on by the attempted double import: https://github.com/inducer/grudge/actions/runs/6062181047/job/16460837326. (Again, builds fine with 7.2.4, breaks with 7.2.5.) We have not yet figured out in detail how the double import breaks things, but... we kind of shouldn't have to? I do not think #11645 can ever be made to work reliably. |
|
Just add an option for default value of |
|
I have something that looks similar to this as well in https://github.com/lab-cosmo/metatensor. The docs builds fine with sphinx 7.2.4, and fails with 7.2.5 with a numpy error that looks related to typechecking: EDIT: we are only seeing this because we build docs with |
|
Well, #11679 is an implementation of my second idea for how to make this work, leveraging I'm not convinced that any of this is a good idea, and I think that leaving But, this new PR does seem to work in more cases than #11645 did. It handles pydantic, at least (I tested that locally, rather than introducing a new test dependency on pydantic). It also passes the tests we added for #11645. I think it's also more likely to leave things in a valid state if the reload fails, by virtue of loading with |
The grudge doc build still fails with #11679, with a new error message: |
|
And it works with Sphinx 7.2.4? That's weird. Hm. Can you give me a recipe I can follow to reproduce that failure myself? I tried cloning https://github.com/inducer/grudge.git and doing a That's with meshmode 2021.2, and grudge's requirement is |
Yes, it works with sphinx 7.2.4. I think the easiest way to install grudge and the correct dependencies might be via emirge: |
|
Concerning the reason why we had this bug, I suggested the following: #11652 (comment) |
|
Well, it looks like the issue with One of my assumptions with the "use So... I'm not sure I have any good ideas at this point. I'm not seeing any reasonable way to import modules with For the sake of anyone stumbling upon this issue, it might be helpful to document that there's a reasonable workaround for both the problems caused by 7.2.0 and the different problems caused by 7.2.5: if you import the modules that you're trying to document yourself from inside your |
|
Posted PyO3/pyo3#3446 - I'll try to ship this within a week and update Pydantic to use. |
See sphinx-doc/sphinx#11662 which seems like it will be resolved with an upstream PyO3 release. May have to come back and change this pin to be an upper pin if need be / the upstream issue doesn't merge soon.
|
7.2.6 does fix the issue in my case (#11662 (comment)). Thanks a lot to all contributors! |
|
Confirmed fixed in 7.2.6. Thanks for the hard work on this! |
Describe the bug
New error message with this patch release. 7.2.4 succeeds in building the same documents.
PyO3 modules may only be initialized once per interpreter processHow to Reproduce
install latest sphinx and run
sphinx-build -WaE docs/project docs/project/_build/htmlEnvironment Information
Sphinx extensions
Additional context
No response
The text was updated successfully, but these errors were encountered: