twisted / twisted

Event-driven networking engine written in Python.
https://twisted.org
Other
5.58k stars 1.17k forks source link

ValueError: signal only works in main thread of the main interpreter #11660

Closed avarf closed 2 years ago

avarf commented 2 years ago

Describe the incorrect behavior you saw I am using twisted in a handler of a web server (FastAPI) and the idea is to for each post request we continuously run a function in the background using the router BUT I am getting this error:

DEBUG:    [pv.py->read_weather_forecast():180] 2022-09-12 17:02:21 | Returning the last message
DEBUG:    [pv.py->get_output():142] 2022-09-12 17:02:21 | weather forecast read from the Kafka -- pass the weather forecast to the model
Unhandled Error
Traceback (most recent call last):
  File "/home/.local/lib/python3.10/site-packages/twisted/internet/base.py", line 498, in fireEvent
    DeferredList(beforeResults).addCallback(self._continueFiring)
  File "/home/.local/lib/python3.10/site-packages/twisted/internet/defer.py", line 497, in addCallback
    return self.addCallbacks(callback, callbackArgs=args, callbackKeywords=kwargs)
  File "/home/.local/lib/python3.10/site-packages/twisted/internet/defer.py", line 477, in addCallbacks
    self._runCallbacks()
  File "/home/.local/lib/python3.10/site-packages/twisted/internet/defer.py", line 857, in _runCallbacks
    current.result = callback(  # type: ignore[misc]
--- <exception caught here> ---
  File "/home/.local/lib/python3.10/site-packages/twisted/internet/base.py", line 510, in _continueFiring
    callable(*args, **kwargs)
  File "/home/.local/lib/python3.10/site-packages/twisted/internet/base.py", line 1311, in _reallyStartRunning
    self._handleSignals()
  File "/home/.local/lib/python3.10/site-packages/twisted/internet/posixbase.py", line 338, in _handleSignals
    _SignalReactorMixin._handleSignals(self)
  File "/home/.local/lib/python3.10/site-packages/twisted/internet/base.py", line 1279, in _handleSignals
    signal.signal(signal.SIGTERM, reactorBaseSelf.sigTerm)
  File "/usr/lib/python3.10/signal.py", line 56, in signal
    handler = _signal.signal(_enum_to_int(signalnum), _enum_to_int(handler))
builtins.ValueError: signal only works in main thread of the main interpreter

Describe how to cause this behavior The code that triggers this error:

The creation of reactor and starting the process

self.r = reactor
self.loop = task.LoopingCall(self.__stream_output)
self.loop.start(
    req.freq
)  # call every freq seconds (freq format: 60.0)
self.r.run()

def __stream_output(self):
    output = self.asset.get_output()

Asset get_output:

from pvlib import modelchain

    def get_output(self):
        logger.debug("Inside PV.get_output. Start reading the weather forecast from kafka.")
        weather = self.read_weather_forecast()
        logger.debug("weather forecast read from the Kafka -- pass the weather forecast to the model")

        self.mc.run_model(weather) # --> Error happens here. This is a modelchain from PVlib. I checked the modelchain but nothing strange happens there and also it is not using multi-threading
        logger.debug("returning the output from the model")
        return self.mc.results.ac
Automated tests that are demonstrating the failure would be awesome.

**Testing environment**
 - Operating System and Version; paste the output of these commands:
```bash
$ uname -a ; cat /etc/lsb-release
Linux ali-P51 5.15.0-43-generic #46-Ubuntu SMP Tue Jul 12 10:30:17 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=22.04
DISTRIB_CODENAME=jammy
DISTRIB_DESCRIPTION="Ubuntu 22.04.1 LTS"
adiroiban commented 2 years ago

Hi.

Thanks for the report.

I am not sure why this is a bug. Is there documentation informing that signals works anywhere?

If you want to start the reactor outside of the main thread you can start the reactor without installing the signal handling.


as far as I know, the whole signaling system is an Unix inter process communication mechanism. and is designed to be handled by the main thread of each process.

is not an inter-thread communication thing.

So I think that you need a custom reactor setup in which does something like this

reactor.run(installSignalHandlers=False)

Would that work?

avarf commented 2 years ago

I tried that but exactly at the point that I was getting the error, no the program hangs. I am looking into this, just one question, is it advised to not use the reactor in the way that I am using it? I mean in the backend of a web server.

adiroiban commented 2 years ago

I don't know much about FastAPI. I imagine FastAPI has its own reactor... and you can only have a single reactor per process.

Twisted wanting the reactor in the main process is a way to make sure you don't end up with conflicts with other reactors.


is it advised to not use the reactor in the way that I am using it? I mean in the backend of a web server.

As long as you know what you are doing and know how Twisted main thread works, it should be fine.

I don't know what "backend of a web server" means.

Do you have a full example that I can run on my side and reproduce the error?

avarf commented 2 years ago

Neither FastAPI nor the PVlib uses a reactor.

What I want to do is to create and continuously run a job when I receive a POST request. By the "backend of a server" I mean as soon as the job has been created, a success response will be sent to the POST request but the job stays alive in a separate thread.

I did a simpler version of this to fetch the weather forecast using schedule and everything works fine there but here I wanted more control so I decided to use reactor.

adiroiban commented 2 years ago

I expect that FastAPI has some sort of reactor, as otherwise, it would not be able to react and handle new connections.

So this looks more like a discussion about how to integrate Twisted with FastAPI ... or FastAPI with Twisted :)

avarf commented 2 years ago

FastAPI itself is not using a reactor but it is relying on Uvicron for multi-threading. I just checked the uvicron git repo and someone had a very similar issue: https://github.com/encode/uvicorn/issues/850

And it seems flask also has the same issue :) I am missing my goroutines and channels :)

exarkun commented 2 years ago

Python itself only supports installing signal handlers in the main thread. Twisted uses signal handlers to support shutdown by Ctrl-C and to know when to clean up after child processes. Twisted can therefore only supply these features when the reactor is run in the main thread. In the context of something like uvicron, the Twisted reactor won't run in the main thread so these features are unavailable. You turn them off as suggested above, with installSignalHandlers=False.

Additionally, Twisted's reactors can only be started once per process. So you cannot call reactor.run once per POST. You need to manage a separate long-lived thread that's dedicated to running the Twisted reactor and then do thread-safe calls between your request threads and that reactor thread.

There is a third-party library to do most of the hard parts of this for you, https://crochet.readthedocs.io/en/stable/api.html

Folks on the mailing list (https://mail.python.org/mailman3/lists/twisted.python.org) or the chat room (https://gitter.im/twisted/twisted) can probably provide further help with the tricky task of integrating Twisted with a thread-based system.