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

allow per-thread atexit() #58281

Open
tarekziade mannequin opened this issue Feb 21, 2012 · 17 comments
Open

allow per-thread atexit() #58281

tarekziade mannequin opened this issue Feb 21, 2012 · 17 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) stdlib Python modules in the Lib dir topic-subinterpreters type-feature A feature request or enhancement

Comments

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Feb 21, 2012

BPO 14073
Nosy @tim-one, @pitrou, @vstinner, @tiran, @tarekziade, @zvezdan, @ericsnowcurrently

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:

assignee = None
closed_at = None
created_at = <Date 2012-02-21.14:12:07.896>
labels = ['expert-subinterpreters', 'interpreter-core', 'type-feature', 'library']
title = 'allow per-thread atexit()'
updated_at = <Date 2020-06-03.16:42:40.636>
user = 'https://github.com/tarekziade'

bugs.python.org fields:

activity = <Date 2020-06-03.16:42:40.636>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core', 'Library (Lib)', 'Subinterpreters']
creation = <Date 2012-02-21.14:12:07.896>
creator = 'tarek'
dependencies = []
files = []
hgrepos = []
issue_num = 14073
keywords = []
message_count = 16.0
messages = ['153871', '153905', '153931', '153932', '153933', '153939', '153940', '153943', '153945', '153947', '204439', '204441', '204446', '204494', '236501', '238383']
nosy_count = 11.0
nosy_names = ['tim.peters', 'pitrou', 'vstinner', 'christian.heimes', 'tarek', 'grahamd', 'zvezdan', 'neologix', 'eric.snow', 'Glenn.Maynard', 'Vadim Markovtsev']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue14073'
versions = ['Python 3.5']

@tarekziade
Copy link
Mannequin Author

tarekziade mannequin commented Feb 21, 2012

If you try to run the code below and stop it with ctrl+C, it will lock because atexit is never reached.

Antoine proposed to add a way to have one atexit() per thread, so we can call some cleanup code when the app shuts down and there are running threads.

from wsgiref.simple_server import make_server
import threading
import time
import atexit

class Work(threading.Thread):
    def __init__(self):
        threading.Thread.__init__(self)
        self.running = False

    def run(self):
        self.running = True
        while self.running:
            time.sleep(.2)

    def stop(self):
        self.running = False
        self.join()

worker = Work()


def shutdown():
    # bye-bye
    print 'bye bye'
    worker.stop()


atexit.register(shutdown)


def hello_world_app(environ, start_response):
    status = '200 OK' # HTTP Status
    headers = [('Content-type', 'text/plain')]
    start_response(status, headers)
    return ["Hello World"]

def main():
    worker.start()
    return make_server('', 8000, hello_world_app)

if __name__ == '__main__':
    server = main()
    server.serve_forever()

@tarekziade tarekziade mannequin added the type-bug An unexpected behavior, bug, or error label Feb 21, 2012
@pitrou pitrou added interpreter-core (Objects, Python, Grammar, and Parser dirs) stdlib Python modules in the Lib dir type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Feb 21, 2012
@grahamd
Copy link
Mannequin

grahamd mannequin commented Feb 21, 2012

My take on this is that if wanting to interact with a thread from an atexit callback, you are supposed to call setDaemon(True) on the thread. This is to ensure that on interpreter shutdown it doesn't try and wait on the thread completing before getting to atexit callbacks.

@tarekziade
Copy link
Mannequin Author

tarekziade mannequin commented Feb 22, 2012

@Grahamd : sometimes you don't own the code that contains the thread, so I think it's better to be able to shutdown properly all flavors of threads.

@grahamd
Copy link
Mannequin

grahamd mannequin commented Feb 22, 2012

Reality is that the way Python behaviour is defined/implemented means that it will wait for non daemonised threads to complete before exiting.

Sounds like the original code is wrong in not setting it to be daemonised in the first place and should be reported as a bug in that code rather than fiddling with the interpreter.

@tarekziade
Copy link
Mannequin Author

tarekziade mannequin commented Feb 22, 2012

Is there any good reason not to add this feature ? what would be the problem ? It does seem to be for the best, I don't see any drawbacks

@grahamd
Copy link
Mannequin

grahamd mannequin commented Feb 22, 2012

At the moment you have showed some code which is causing you problems and a vague idea. Until you show how that idea may work in practice it is a bit hard to judge whether what it does and how it does it is reasonable.

@tarekziade
Copy link
Mannequin Author

tarekziade mannequin commented Feb 22, 2012

Mmm.. you did not say yet why you are against this feature, other than "the lib *should not* use non-daemonized threads"

This sounds like "the lib should not use feature X in Python because it will block everything"

And now we're proposing to remove the limitation and you are telling me I am vague and unreasonable.

Let me try differently then.

Consider this script to be a library I don't control. I need to call the .stop() function when my main application shuts down.

I can't use signals because you forbid it in mod_wsgi

How do I do, since asking the person to daemonize his thread is not an option ?

I see several options:
1 - monkey patch the lib
2 - remove regular threads from Python, or making them always daemonized
3 - add an atexit() option in threads in Python
4 - use signals and drop the usage of mod_wsgi

I think 3- is the cleanest.

@grahamd
Copy link
Mannequin

grahamd mannequin commented Feb 22, 2012

I haven't said I am against it. All I have done so far is explain on the WEB-SIG how mod_wsgi works and how Python currently works and how one would normally handle this situation by having the thread be daemonised.

As for the proposed solution, where is the code example showing how what you are suggesting is meant to work. Right now you are making people assume how that would work. Add an actual example here at least of how with the proposed feature your code would then look.

For the benefit of those who might even implement what you want, which will not be me anyway as I am not involved in Python core development, you might also explain where you expect these special per thread atexit callbacks to be triggered within the current steps for shutting down the interpreter. That way it will be more obvious to those who come later as to what you are actually proposing.

@tarekziade
Copy link
Mannequin Author

tarekziade mannequin commented Feb 22, 2012

Add an actual example here at least of how with the proposed feature your code would then look.

That's the part I am not sure at all about in fact. I don't know at all the internals in the shutdown process in Python and I was hoping Antoine would give us a proposal here.

I would suspect simply adding to the base thread class an .atexit() method that's called when atexit() is called, would do the trick since we'd be able to do things like:

  def atexit(self):
      ... do whatever cleanup needed...
      self.join()

but I have no real experience in these internals.

@grahamd
Copy link
Mannequin

grahamd mannequin commented Feb 22, 2012

Except that calling it at the time of current atexit callbacks wouldn't change the current behaviour. As quoted in WEB-SIG emails the sequence is:

wait_for_thread_shutdown();

/* The interpreter is still entirely intact at this point, and the
 * exit funcs may be relying on that.  In particular, if some thread
 * or exit func is still waiting to do an import, the import machinery
 * expects Py_IsInitialized() to return true.  So don't say the
 * interpreter is uninitialized until after the exit funcs have run.
 * Note that Threading.py uses an exit func to do a join on all the
 * threads created thru it, so this also protects pending imports in
 * the threads created via Threading.
 */
call_py_exitfuncs();

So would need to be done prior to wait_for_thread_shutdown() or by that function before waiting on thread.

The code in that function has:

    PyObject *threading = PyMapping_GetItemString(tstate->interp->modules,
                                                  "threading");
...
result = PyObject_CallMethod(threading, "_shutdown", "");

So calls _shutdown() on the threading module.

That function is aliased to _exitfunc() method of _MainThread.

    def _exitfunc(self):
        self._stop()
        t = _pickSomeNonDaemonThread()
        if t:
            if __debug__:
                self._note("%s: waiting for other threads", self)
        while t:
            t.join()
            t = _pickSomeNonDaemonThread()
        if __debug__:
            self._note("%s: exiting", self)
        self._delete()

So can be done in here.

The decision which would need to be made is whether you call atexit() on all threads before then trying to join() on any, or call atexit() only prior to the join() of the thread.

Calling atexit() on all possibly sounds the better option but I am not sure, plus the code would need to deal with doing two passes like that which may not may not have implications.

@GlennMaynard
Copy link
Mannequin

GlennMaynard mannequin commented Nov 25, 2013

This would be useful. It shouldn't be part of atexit, since atexit.register() from a thread should register a process-exit handler; instead, something like threading.(un)register_atexit(). If called in a thread, the calls happen when run() returns; if called in the main thread, call them when regular atexits are called (perhaps interleaved with atexit, as if atexit.register had been used).

For example, this can be helpful to handle cleaning up per-thread singletons like database connections.

@tiran
Copy link
Member

tiran commented Nov 25, 2013

A couple of years ago I suggested something similar. I'd like to see both thread_start and thread_stop hooks so code can track the creation and destruction of threads. It's a useful feature for e.g. PyLucene or profilers. The callback must be inside the thread and not from the main thread, though.

Perhaps somebody likes to work on a PEP for 3.5?

@pitrou
Copy link
Member

pitrou commented Nov 26, 2013

Most logical would be an API on Thread objects (this would obviously only work with threading-created threads). A PEP also sounds unnecessary for a single new API.

@vstinner
Copy link
Member

See also the issue bpo-19466: the behaviour of daemon threads changed at Python exit in Python 3.4.

@BreamoreBoy
Copy link
Mannequin

BreamoreBoy mannequin commented Feb 24, 2015

The changes referenced in msg204494 ref: bpo-19466 were reverted via changesets 9ce58a73b6b5 and 1166b3321012

@VadimMarkovtsev
Copy link
Mannequin

VadimMarkovtsev mannequin commented Mar 18, 2015

I agree that there must be some way to join the threads before exiting, with a callback or anything else. Currently, my thread pool implementation has to monkey patch sys.exit and register SIGINT handler to shutdown itself and avoid the hangup (100+ LoC to cover all possible exceptions). I am working on a big framework and demanding from users to call "thread pool shutdown" function before exit would be yet another thing they must remember and just impossible in some cases. It would ruin the whole abstraction. Python is not C, you know.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@ezio-melotti ezio-melotti moved this to Todo in Subinterpreters Apr 15, 2022
@gpshead
Copy link
Member

gpshead commented May 26, 2023

Threads are not guaranteed to exit and cannot be safely forced to do so. Even non-daemon threads. Any thread can be blocked on something beyond our control.

As an aside: we don't recommend anyone actually use the old daemon thread concept anymore as they interact poorly with interpreter shutdown.

regardless, for Python launched threads that do exit cleanly agreed that a stop/exit notification for those could be useful. But we need to make it clear to people it cannot be guaranteed to happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) stdlib Python modules in the Lib dir topic-subinterpreters type-feature A feature request or enhancement
Projects
Status: Todo
Status: No status
Development

No branches or pull requests

4 participants