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
module shutdown procedure based on GC #39300
Comments
This patches changes PyImport_Cleanup() in an attempt http://mail.python.org/pipermail/python-dev/2003-September/038238.html To prevent objects with __del__ methods from keeping Note that weak references to dead cycles containing This patch does not change the behavior of module This patch puts weak references (actually proxies) in And finally -- this patch has not really been tested, |
Logged In: YES I think clearing gc.garbage at shutdown time is a good idea; |
Logged In: YES Not sure if this is a good idea, but I wonder if the extensions I suppose _PyImport_Fini is called when it is called because |
Logged In: YES Just so you know, Armin, the patch did not apply cleanly; the comment |
Consider this patch for 2.6 and discuss it at the bug day. |
Looking at the patch, is there any reason it doesn't get rid of the The major annoyance with the current scheme is that, at interpreter About what to do of gc.garbage at shutdown, there was another proposal |
This sounds like a nice idea. The current cleanup procedure in I've updated the patch to work with the current SVN head. Probably this |
Retargetting, and I hope someone can take a look at the patch and give |
Does this patch fix bpo-1545463 by any chance? I am away from a |
It should fix bpo-1545463 and running a quick test seems to show that |
bpo-1545463 has been closed as "won't fix", so wouldn't implementing this patch kill two birds with one stone? |
bpo-1545463 has been reopened with comments about being used as a stop gap. Possibly review and implementation of the patch here would be a better option, sorry it's over my head. |
0001-update-GC-shutdown-patch.patch looks sane to me at first glance. any other opinions? |
The patch is obviously against 2.x (there are some PyString_Check's on module names, for example). It should be regenerated against 3.x. Also, it would be nice if a test could be devised to check that the shutdown procedure works as expected (I'm not sure how such a test would look like). |
In bpo-14035, Florent pointed out the current behaviour potentially causes problems for some uses of import_fresh_modules() in the test suite (with globals sometimes being set to None if there's no indepenent reference to the module). GC based module cleanup would avoid that problem automatically. |
Fwiw, the behavior in PyPy is: don't do anything particular at shut-down, just shut down and quit the process. No hacking at module globals to replace them with None, but also no guaranteeing that any __del__ method is ever called. We didn't get a particular bug report about this so far, so it seems to work. (This is just a report about PyPy's situation; I understand that the situation in CPython is a bit more delicate if CPython is embedded in a larger process.) |
Also, since this issue was last updated, Antoine devised a scheme to test some of the embedding functionality (mainly to test subinterpreters, IIRC). Perhaps that could be harnessed to check GC-based shutdown is working correctly (it might even do it already, without any changes to the test suite). |
I think that would indeed be unacceptable for Python - there is a |
Obviously we run atexit code too. There is no point in having atexit if it's not guaranteed to run in a normal shutdown. |
What is the status of this? Does the patch need more reviewing? |
At the moment, it's like that the status of the patch needs to be reestablished. Does it apply? Does it work? Does the test suite still pass? |
It's been quite a long time since I played with this patch so my memory might be a bit fuzzy. As I recall, it sounds good in theory but in practice it doesn't really work. One of the core problems is that many extension modules keep references to Python objects in global or static variables. These references keep pretty much everything alive and prevent GC cleanup of modules. So, a necessary condition to this working is to get rid of those references and use the new module struct facility introduced by Martin. That would be a huge amount of work but I think should be the long term goal. |
In addition to the problem Neil noted with references in extension modules keeping module objects themselves alive, Antoine recently noted that the other major challenge is the reference cycles between module global dictionaries and their contents. As soon as a module global has both a __del__ method and a reference back to the module globals, the entire cycle becomes uncollectable. I suspect one of the reasons PyPy can cope without the explicit reference breaking step is that their GC is better able to cope with __del__ methods than ours. I wonder if a useful interim step might be to make the current explicit reference breaking hack a bit smarter by looking at the reference counts. (Note: some aspects of this idea could be made simpler if modules supported weak references)
Throughout the process, keep an eye on gc.garbage - if we see a module dict show up there, hit it with the "set all globals to None" hammer. (The new callback functionality in 3.3 makes that easier - for example, you could put a sentinel object in the globals of the module being cleared and watching for a dict containing that sentinel object showing up in 'uncollectable' during the stop phase) |
Superceded by patch in bpo-18214. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: