pytest: Finalizers don't run on SIGTERM

this also wouldn’t be terribly hard to do as a fixture or plugin without adding this to core:

@pytest.fixture(scope='session', autouse=True)
def term_handler():
    orig = signal.signal(signal.SIGTERM, signal.getsignal(signal.SIGINT))
    yield
    signal.signal(signal.SIGTERM, orig)

@asottile pytest is a test runner that does resource management - any python service/tool that does resource management is typically expected to handle the term signal and clean up

so i believe pytest should do so as well

@asottile If finalizers are specifically the sticking point, then I suggest that the topic be switched to the essence of the desire, which is mine as well: if a fixture is in charge of a resource’s lifecycle, then how can resource leaks be avoided?

Why is this any of pytest’s concern? Fixtures provide the main mechanism by which tests that leverage the pytest framework set up their resources. It is even documented to be useful for this purpose:

@pytest.fixture(scope="module", params=["smtp.gmail.com", "mail.python.org"])
def smtp_connection(request):
    smtp_connection = smtplib.SMTP(request.param, 587, timeout=5)
    yield smtp_connection
    print("finalizing {}".format(smtp_connection))
    smtp_connection.close()

Many of these resources are important to clean up. For example, if I create an object in a bucket in Object Storage for my CI job, then it is essential to know that that specific object (and bucket) resource will be cleaned up if that job is canceled mid-run; otherwise, over time, I will accumulate objects and buckets from canceled CI jobs. If I am using the pytest framework and I use a fixture to create that resource, then it is only natural to expect fixtures to have a mechanism, any mechanism at all, to clean up that resource or suddenly fixtures are no longer useful for resource lifecycles in many cases and the documentation is misleading.

SIGTERM is commonplace for canceling a process gracefully. No one disputes that because it’s been that way for a long time. Generally, there is a grace period after the SIGTERM during which the process can clean up and, after that, the SIGTERM is followed up by a SIGKILL. For reference, one can check out what Google does for Kubernetes.

So, since there are certain points that cannot be argued against:

1. pytest documentation recommends fixtures for resource lifecycles
2. SIGTERM is used for graceful termination of a process
3. pytest fixtures for resources can cause that resource to leak when SIGTERM is received
4. Resource leaks are bugs

That leaves only the final question which is: how should resource leaks be avoided by pytest users when a SIGTERM is sent?

@asottile I disagree with it not being the standard behaviour for other programs. I think it’s the opposite, that’s why I opened this ticket. A wrapper doesn’t seem like the right way to handle this. Any process should implement signal handling for SIGTERM to do cleanup imo.

SIGTERM is a signal to the process that it should exit. The point of telling the process it should exit instead of just killing it is to let the process do pre-exit execution such as cleanup. If I wanted to kill the process without cleanup I would send SIGKILL.

@blueyed The kernel closes file descriptors on process exit. I think it makes sense to only do cleanup and nothing else on SIGTERM. I could maybe pick up the work in the future but I don’t have time at the moment. So if you or anyone else want to do the work I’m all for it.

This is standard behaviour in python (and as far as I can tell in most programming languages) – I think it would be more suprising for pytest to implement this.

If you want something that can make this work for you, you could rewrite the signal to SIGINT using a wrapper and then you’d trigger the normal cleanup path for interrupt in python.

One way you could do this is to use dumb-init to rewrite the signal using --rewrite

Here’s a simple example where I turn SIGTERM into SIGINT:

$ ./venv/bin/dumb-init --rewrite 15:2 python3 -c 'import time; time.sleep(1000)'

# I pressed enter a few times and then in another terminal tab I did `pkill -TERM dumb-init`

Traceback (most recent call last):
  File "<string>", line 1, in <module>
KeyboardInterrupt

Closing then, thanks everyone and thanks @gsmethells for opening the follow up issues. 👍

I just reviewed the docs,

I believe pytest should raise a exception and do a graceful shutdown

A correct solution will need a way to sort out blocked teardowns

This may require quite some investigation

I appreciate you took the time to write an example of a workaround. Unfortunately it doesn’t work with multiple threads (tested with xdist). Maybe this is trickier than I first thought.

Probably needs a check for the main thread.

Why do you think finalizers should be run for SIGINT and not SIGTERM?

My fundamental reasoning is that python does not do this, we should not deviate from the standard language behaviour as this introduces a difference in the test environment which is not present in the runtime environment. Additionally, it represents some pretty-complex interaction and cross-platform concerns that we now have to maintain the complexity for.

I have yet to see a programming language which calls finalizers on SIGTERM, only on SIGINT. If anything this is something a language should implement (or individual applications which wish to handle SIGTERM) and not the concern of a testing framework.