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

sub-interpreters: datetime segfaults if imported from two sub-interpreters #110415

Closed
costasgambit opened this issue Oct 5, 2023 · 13 comments
Closed
Assignees
Labels
topic-subinterpreters type-bug An unexpected behavior, bug, or error

Comments

@costasgambit
Copy link

costasgambit commented Oct 5, 2023

Bug report

Bug description:

I've started putting a branch together that moves all the global state into a module-level state object.

If that looks useful, I'll continue to work on it and can rebase on main.

It passes tests and "works on my machine".

I haven't looked at python internals in some time, so apologies if I'm a little rusty!

CPython versions tested on:

3.12, CPython main branch

Operating systems tested on:

Linux

Linked PRs

@costasgambit costasgambit added the type-bug An unexpected behavior, bug, or error label Oct 5, 2023
@gaogaotiantian
Copy link
Member

Sorry but this does not look like a bug report. The content of the issue is a bit unrelated to the title.

Could you paste your code in a minimum reproducible example so we can track it down?

@costasgambit
Copy link
Author

Sorry @gaogaotiantian, here is a bit of code that should demonstrate the segfault on 3.12+:

import _xxsubinterpreters

interp1 = _xxsubinterpreters.create()
interp2 = _xxsubinterpreters.create()

_xxsubinterpreters.run_string(interp1, "import datetime")
_xxsubinterpreters.run_string(interp2, "import datetime")   # crash

@gaogaotiantian
Copy link
Member

I'm not able to repro it with 3.12.0 + WSL(Ubuntu 20.04). Are you using the official release? What's your python -V?

@gaogaotiantian
Copy link
Member

I am able to repro it on main with --with-pydebug.

@costasgambit
Copy link
Author

Python 3.12.0+, but main is the same.

If you scan over _datetime_exec() you'll see there's global state everywhere.

@erlend-aasland
Copy link
Contributor

The datetime module is not fully isolated. I started working on it last year, but I took a break from module isolation work around the 3.12 feature freeze, and I haven't found the energy needed to restart that work. You can take a look at the existing issue/PR:

@erlend-aasland
Copy link
Contributor

Duplicate of #71587

@erlend-aasland erlend-aasland marked this as a duplicate of #71587 Oct 6, 2023
@erlend-aasland
Copy link
Contributor

The major pain point of isolating _datetime is that it has an exposed C API (though datetime.h, which must be explicitly included). I discussed this with @pganssle at the Language Summit and it was not immediately obvious if the datetime.h C API is bound by PEP-387 in the same way API exposed by Python.h is. This point must be resolved before the _datetime module can be completely isolated.

@erlend-aasland
Copy link
Contributor

Also, I prefer a more structured approach similar to my draft PR; a large scale change like #110420 is not optimal wrt. debugging and bisecting. I prefer to implement such changes in more manageable batches. See for example the PRs related to isolating _io, _sqlite, or _decimal.

@encukou
Copy link
Member

encukou commented Oct 6, 2023

Well, you might want to ask the Steering Council for confirmation, but if third-party libraries are expected to use it, then I'd call it C-API -- which means it's covered by PEP-387.

@erlend-aasland
Copy link
Contributor

Meta: either we close this issue as a duplicate and continue the discussion on the original issue (gh-71587), or we close the original issue as superseded. I'm +1 for the former.

@costasgambit
Copy link
Author

I couldn't find the old bug when I had looked for one, and this was a nice excuse for me to dip my toes in the code a little bit.

Very happy to wait for the old issue/PR to be resurrected. I was considering sub-interpreters for something we wanted to do at work, but there's sadly far too many blockers like this for that to be feasible right now.

Also happy to contribute in the other discussion in my own time, I find the isolated sub-interpreters to be quite a promising idea.

@github-project-automation github-project-automation bot moved this from Todo to Done in Subinterpreters Oct 6, 2023
@erlend-aasland erlend-aasland closed this as not planned Won't fix, can't repro, duplicate, stale Oct 6, 2023
@erlend-aasland
Copy link
Contributor

I couldn't find the old bug when I had looked for one [...]

Well, it is not easy to spot the old issue; the symptom is not the same, but OTOH the prescription is :) I started working with Victor on the old PR. It should be possible to split it up in manageable sizes relatively quickly. I'll ping you if you want to join.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-subinterpreters type-bug An unexpected behavior, bug, or error
Projects
Archived in project
Status: Done
Development

No branches or pull requests

6 participants