Issue1680961
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2007-03-14 19:51 by georg.brandl, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
atexit.diff | georg.brandl, 2007-03-14 19:51 | patch | ||
atexit.patch | collinwinter, 2007-03-16 22:02 | Alternate atexit implementation, v3 |
Messages (21) | |||
---|---|---|---|
msg52197 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-14 19:51 | |
This removes sys.exitfunc. atexit is rewritten in C and uses as much of the existing sys.exitfunc-infrastructure (which is renamed). |
|||
msg52198 - (view) | Author: Alan McIntyre (alanmcintyre) * | Date: 2007-03-14 21:30 | |
The attached patch didn't apply cleanly for me; the hunk that removes call_sys_exitfunc (pythonrun.c) failed, but everything else applied just fine. I removed call_sys_exitfunc manually and then everything compiled and passes regrtest.py (with -r -u all) with no unexpected skips (Linux). The docs also build just fine and I don't see any grammatical/formatting errors or anything that jumps out as a technical error (but I'm not all that familiar with atexit/exitfunc, so take my endorsement with a grain of salt ;). |
|||
msg52199 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-14 22:30 | |
Note that the patch is against the p3yk branch. Note also that, without changing Modules/Setup, the new module won't be built and used. In fact, the test suite doesn't pass with it since I haven't updated the tests that depend on atexit._exithandlers. But thanks for reviewing nonetheless! |
|||
msg52200 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-15 06:11 | |
Ack, I didn't know you were working on this, too! I'm attaching my own atexit implementation; the patch includes changes to Modules/Setup, test_atexit and test___all__ so that the test suite passes. One thing I'm not sure about is that I've moved call_ll_exitfuncs() up in the order of Py_Finalize(). This was necessary to make PyThreadState_GET() stop complaining about "no current thread" when calling the atexit functions at interpreter-shutdown, but I don't know enough to be sure that this change doesn't alter some assumption about finalization order. It looks alright to me, and "make test" passes, but I'd appreciate it if someone could look over that part. File Added: atexit.patch |
|||
msg52201 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-15 07:28 | |
To summarize the differences: your code uses a C array to store the exit handlers, mine uses a Python list. Which to choose obviously depends on whether you want to make the list available to Python code... |
|||
msg52202 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-15 16:49 | |
Another difference: mine uses Py_AtExit(), yours adds extra C functions like Py_SetPythonExitfunc() and call_python_exitfunc(). I'd vote against exposing the handler list to Python code. I see that leading to "run my handler first! No, mine! *scuffle*" games. |
|||
msg52203 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-15 17:00 | |
What about a function to get a *copy* of the exitfuncs? (Okay, I can't see a use case right now, but maybe it helps transitioning code that inspected atexit._exithandlers before) |
|||
msg52204 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-15 17:22 | |
Googling turns up that the main use of atexit._exithandlers is to a) unregister a callback or b) clear all callbacks. I've already provided a _clear() function to handle (b), and an unregister() to deal with (a) shouldn't be hard. Thoughts? |
|||
msg52205 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-15 18:01 | |
Yes, that would work for me. The issue I see with moving the low-level exitfuncs that is that the functions registered there may make resources unavailable that are necessary for Python code to run (and of course, atexit handlers can run arbitrary Python code...) |
|||
msg52206 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-15 20:38 | |
Isn't that (unavailable resources) a problem with atexit handlers in general, regardless of where they occur in Py_Finalize()? All existing low-level exitfuncs are already guaranteed not to mess with interpreter/thread state, simply because they can't (as things stand). If exitfuncs written against 3.0+ start messing around with Python's internal state, that's a bug in the exitfunc. We're all adults, etc. I'm attaching the new version of my patch, which includes atexit.unregister(). File Added: atexit.patch |
|||
msg52207 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-15 21:14 | |
For example, Modules/socketmodule.c's Py_AtExit handler, for Windows, calls WSACleanup(), after which, if I understand the docs correctly, one can't use sockets anymore. That would mean that if atexit is imported before socket, all atexit-registered exitfuncs cannot use sockets. |
|||
msg52208 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-15 21:27 | |
Doesn't that problem already exist, though, for C extensions that need sockets (or other resources) in their Py_AtExit handlers? Would changing call_ll_exitfuncs() from FIFO to LIFO (like atexit is) help this? |
|||
msg52209 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-15 21:35 | |
Current AtExit handlers are not likely to use functionality not in their domain, though. The socket one just cleans up the socket API, the Tkinter one (which is disabled though) cleans up Tcl, etc. I thought call_ll_exitfuncs() is already LIFO. |
|||
msg52210 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-15 21:45 | |
It is LIFO; I was going off memory, sorry. Is your assumption that atexit.py handlers will be more likely to use functionality outside their domain than would Py_AtExit handlers? |
|||
msg52211 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-16 08:28 | |
It's not specified that exit handlers can do cleanup only, in theory they could do things like sending an email etc. Please let's not introduce gratuitous breakage here. |
|||
msg52212 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-16 22:02 | |
Ok, I'm sold on the two-phase atexit structure. I've updated my patch to keep atexit's handlers separate from the call_ll_exitfuncs() handlers. File Added: atexit.patch |
|||
msg52213 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-17 16:14 | |
Looks good, four points: - should _clear keep its underscore? - unregistering will fail with bound methods, don't know if we should do something about that - you messed up the indentation in pythonrun.c and atexit.c (spaces in the former and tabs in the latter) ;) - do we need docs for Py_PyAtExit? If not, it should be nonpublic and get a leading underscore. |
|||
msg52214 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-18 01:54 | |
- I think _clear() should keep the underscore; it's probably not something that people should be encouraged to use. - I've fixed bound method support. - I'm going to make Py_PyAtExit non-public. |
|||
msg52215 - (view) | Author: Georg Brandl (georg.brandl) * | Date: 2007-03-19 19:13 | |
You didn't attach a patch, but if you do these modification, you can check it in directly IMO. |
|||
msg52216 - (view) | Author: Collin Winter (collinwinter) * | Date: 2007-03-21 02:58 | |
Checked in as r54472. |
|||
msg195585 - (view) | Author: Antoine Pitrou (pitrou) * | Date: 2013-08-18 23:06 | |
Sorry to ping you on an old issue, but can any of you explain why it was necessary to rewrite atexit in C? |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:56:23 | admin | set | github: 44715 |
2013-08-18 23:08:49 | doughellmann | set | nosy:
+ doughellmann |
2013-08-18 23:06:09 | pitrou | set | nosy:
+ pitrou messages: + msg195585 resolution: accepted -> fixed stage: resolved |
2008-01-06 22:29:46 | admin | set | keywords:
- py3k versions: + Python 3.0 |
2007-03-14 19:51:10 | georg.brandl | create |