Age | Commit message (Collapse) | Author | Files | Lines |
|
This adds a check to nxt_python_asgi_get_event_loop() on the
event_loop_func name in the case that running that function fails, and
if it's get_running_loop() that failed we skip printing an error message
as this is an often expected behaviour since the previous commit and we
don't want users reporting erroneous bugs.
This check will always happen regardless of Python version while it
really only applies to Python >= 3.7, there didn't seem much point
adding complexity to the code for this case and in what will be an ever
diminishing case of people running older Pythons.
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Several users on GitHub reported issues with running Python ASGI apps on
Unit with Python 3.11.1 (this would also effect Python 3.10.9) with the
following error from Unit
2023/01/15 22:43:22 [alert] 0#77128 [unit] Python failed to call 'asyncio.get_event_loop'
TL;DR
asyncio.get_event_loop() is currently broken due to the process of
deprecating part or all of it.
First some history.
In Unit we had this commit
commit 8dcb0b9987033d0349a6ecf528014a9daa574787
Author: Max Romanov <max.romanov@nginx.com>
Date: Thu Nov 5 00:04:59 2020 +0300
Python: request processing in multiple threads.
One of things this did was to create a new asyncio event loop in each
thread using asyncio.new_event_loop().
It's perhaps worth noting that all these asyncio.* functions are Python
functions that we call from the C code in Unit.
Then we had this commit
commit f27fbd9b4d2bdaddf1e7001d0d0bc5586ba04cd4
Author: Max Romanov <max.romanov@nginx.com>
Date: Tue Jul 20 10:37:54 2021 +0300
Python: using default event_loop for main thread for ASGI.
This changed things so that Unit calls asyncio.get_event_loop() in the
_main_ thread (but still calls asyncio.new_event_loop() in the other
threads).
asyncio.get_event_loop() up until recently would either return an
already running event loop or return a newly created one.
This was done for $reasons that the commit message and GitHub issue #560
hint at. But the intimation is that there can already be an event loop
running from the application (I assume it's referring to the users
application) at this point and if there is we should use it.
Now for the Python side of things.
On the main branch we had
commit 172c0f2752d8708b6dda7b42e6c5a3519420a4e8
Author: Serhiy Storchaka <storchaka@gmail.com>
Date: Sun Apr 25 13:40:44 2021 +0300
bpo-39529: Deprecate creating new event loop in asyncio.get_event_loop() (GH-23554)
This commit began the deprecating of asyncio.get_event_loop().
commit fd38a2f0ec03b4eec5e3cfd41241d198b1ee555a
Author: Serhiy Storchaka <storchaka@gmail.com>
Date: Tue Dec 6 19:42:12 2022 +0200
gh-93453: No longer create an event loop in get_event_loop() (#98440)
This turned asyncio.get_event_loop() into a RuntimeError _if_ there
isn't a current event loop.
commit e5bd5ad70d9e549eeb80aadb4f3ccb0f2f23266d
Author: Serhiy Storchaka <storchaka@gmail.com>
Date: Fri Jan 13 14:40:29 2023 +0200
gh-100160: Restore and deprecate implicit creation of an event loop (GH-100410)
This re-creates the event loop if there wasn't one and emits a
deprecation warning.
After at least the last two commits Unit no longer works with the Python
_main_ branch.
Meanwhile on the 3.11 branch we had
commit 3fae04b10e2655a20a3aadb5e0d63e87206d0c67
Author: Serhiy Storchaka <storchaka@gmail.com>
Date: Tue Dec 6 17:15:44 2022 +0200
[3.11] gh-93453: Only emit deprecation warning in asyncio.get_event_loop when a new event loop is created (#99949)
which is what caused our breakage, though perhaps unintentionally as we
get the following traceback
Traceback (most recent call last):
File "/usr/lib64/python3.11/asyncio/events.py", line 676, in get_event_loop
f = sys._getframe(1)
^^^^^^^^^^^^^^^^
ValueError: call stack is not deep enough
2023/01/18 02:46:10 [alert] 0#180279 [unit] Python failed to call 'asyncio.get_event_loop'
However, regardless, it is clear we need to stop using
asyncio.get_event_loop().
One option is to switch to the higher level asyncio.run() API, however
that is a rather large change.
This commit takes the simpler approach of using
asyncio.get_running_loop() (which it seems get_event_loop() will
eventually be an alias of) in the _main_ thread to return the currently
running event loop, or if there is no current event loop, it will call
asyncio.new_event_loop() to return a newly created event loop.
I believe this mimics the current behaviour. In my testing
get_event_loop() seemed to always return a newly created loop, as when
just calling get_running_loop() it would return NULL and we would fail
out.
When running two processes each with 2 threads we would get the
following loops with Python 3.11.0 and unpatched Unit
<_UnixSelectorEventLoop running=False closed=False debug=False>
<_UnixSelectorEventLoop running=False closed=False debug=False>
<_UnixSelectorEventLoop running=False closed=False debug=False>
<_UnixSelectorEventLoop running=False closed=False debug=False>
and with Python 3.11.1 and a patched Unit we would get
<_UnixSelectorEventLoop running=False closed=False debug=False>
<_UnixSelectorEventLoop running=False closed=False debug=False>
<_UnixSelectorEventLoop running=False closed=False debug=False>
<_UnixSelectorEventLoop running=False closed=False debug=False>
Tested-by: Rafał Safin <rafal.safin12@gmail.com>
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This is a preparatory patch that factors out the asyncio event loop
creation code from nxt_python_asgi_ctx_data_alloc() into its own
function, to facilitate being called multiple times.
This a part of the work to move away from using the
asyncio.get_event_loop() function due to it no longer creating event
loops if there wasn't one running.
See the following commit for the gory details.
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
There was a couple of reports of Python applications failing due to the
following type of error
File "/opt/netbox/netbox/netbox/configuration.py", line 25, in _import
print(f"\U0001f9ec loaded config '{path}'")
UnicodeEncodeError: 'ascii' codec can't encode character '\U0001f9ec' in
position 0: ordinal not in range(128)
due to the use of Unicode text in the print() statement.
This only happened for python 3.8+ when using the "home" configuration
option as this meant we were going through the new PyConfig
configuration.
When using this new configuration method with the 'isolated' specific
API (for embedded Python) UTF-8 is disabled by default,
PyPreConfig->utf8_mode = 0.
To fix this we need to setup the Python pre config and enable utf-8
mode. However rather than enable utf-8 unconditionally we can set to it
to -1 so that it will use the LC_CTYPE environment variable to determine
whether to enable utf-8 mode or not. utf-8 mode will be enabled if
LC_CTYPE is either: C, POSIX or some specific UTF-8 locale. This is the
default utf8_mode setting when using the non-isolated PyPreConfig API.
Reported-by: Tobias Genannt <tobias.genannt@kappa-velorum.net>
Tested-by: Tobias Genannt <tobias.genannt@kappa-velorum.net>
Link: <https://peps.python.org/pep-0587/>
Link: <https://docs.python.org/3/c-api/init_config.html#c.PyPreConfig.utf8_mode>
Fixes: 491d0f70 ("Python: Added support for Python 3.11.")
Closes: <https://github.com/nginx/unit/issues/817>
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This is a preparatory patch for future work and cleans up the code a
little in the Python 3.8+ variant of nxt_python3_init_config().
The main advantage being we no longer have calls to PyConfig_Clear() in
two different paths.
The variables have a little extra space in their declarations to allow
for the next patch which introduces a variable with a longer type name,
which will help reduce the size of the diff.
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This patch gives users the option to set a `"prefix"` attribute
for Python applications, either at the top level or for specific
`"target"`s. If the attribute is present, the value of `"prefix"`
must be a string beginning with `"/"`. If the value of the `"prefix"`
attribute is longer than 1 character and ends in `"/"`, the
trailing `"/"` is stripped.
The purpose of the `"prefix"` attribute is to set the `SCRIPT_NAME`
context value for WSGI applications and the `root_path` context
value for ASGI applications, allowing applications to properly route
requests regardless of the path that the server uses to expose the
application.
The context value is only set if the request's URL path begins with
the value of the `"prefix"` attribute. In all other cases, the
`SCRIPT_NAME` or `root_path` values are not set. In addition, for
WSGI applications, the value of `"prefix"` will be stripped from
the beginning of the request's URL path before it is sent to the
application.
Reviewed-by: Andrei Zeliankou <zelenkov@nginx.com>
Reviewed-by: Artem Konev <artem.konev@nginx.com>
Signed-off-by: Alejandro Colomar <alx@nginx.com>
|
|
Python 3.8 added a new Python initialisation configuration API[0].
Python 3.11 marked the old API as deprecated resulting in the following
compiler warnings which we treat as errors, failing the build
src/python/nxt_python.c: In function ‘nxt_python_start’:
src/python/nxt_python.c:130:13: error: ‘Py_SetProgramName’ is deprecated [-Werror=deprecated-declarations]
130 | Py_SetProgramName(nxt_py_home);
| ^~~~~~~~~~~~~~~~~
In file included from /opt/python-3.11/include/python3.11/Python.h:94,
from src/python/nxt_python.c:7:
/opt/python-3.11/include/python3.11/pylifecycle.h:37:38: note: declared here
37 | Py_DEPRECATED(3.11) PyAPI_FUNC(void) Py_SetProgramName(const wchar_t *);
| ^~~~~~~~~~~~~~~~~
src/python/nxt_python.c:134:13: error: ‘Py_SetPythonHome’ is deprecated [-Werror=deprecated-declarations]
134 | Py_SetPythonHome(nxt_py_home);
| ^~~~~~~~~~~~~~~~
/opt/python-3.11/include/python3.11/pylifecycle.h:40:38: note: declared here
40 | Py_DEPRECATED(3.11) PyAPI_FUNC(void) Py_SetPythonHome(const wchar_t *);
| ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
We actually have a few config scenarios: Python < 3, Python >= 3.0 < 3.8
and for Python 3 we have two configs where we select one based on
virtual environment setup.
Factor out the Python 3 config initialisation into its own function. We
actually create two functions, one for Python 3.8+ and one for older
Python 3. We pick the right function to use at build time.
The new API also has error checking (where the old API doesn't) which we
handle.
[0]: https://peps.python.org/pep-0587/
Closes: <https://github.com/nginx/unit/issues/710>
[ Andrew: Expanded upon patch from @sandeep-gh ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
Splitting `nxt_python_add_sptr` into several functions will make future
additions easier.
Signed-off-by: Alejandro Colomar <alx@nginx.com>
|
|
Signed-off-by: Alejandro Colomar <alx@nginx.com>
|
|
|
|
This is a preparatory patch that renames the 'local' and 'local_length'
members of the nxt_unit_request_t structure to 'local_addr' and
'local_addr_length' in preparation for the adding of 'local_port' and
'local_port_length' members.
Suggested-by: Zhidao HONG <z.hong@f5.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
This change was forgotten in the original implementation 282123ba4f7b.
|
|
This closes #635 issue on GitHub.
|
|
The previous commit added more generic APIs for handling
NXT_CONF_VALUE_ARRAY and non-NXT_CONF_VALUE_ARRAY together.
Modify calling code to remove special cases for arrays and
non-arrays, taking special care that the path for non arrays is
logically equivalent to the previous special cased code.
Use the now-generic array code only.
|
|
Introduced in the 78864c9d5ba8 commit.
Sorry about that.
|
|
The __call__ method can be native and not be a PyFunction type. A type check
is thus required before accessing op_code and other fields.
Reproduced on Ubuntu 21.04, Python 3.9.4 and Falcon framework: here, the
App.__call__ method is compiled with Cython, so accessing op_code->co_flags is
invalid; accidentally, the COROUTINE bit is set which forces the Python module
into the ASGI mode.
The workaround is explicit protocol specification.
Note: it is impossible to specify the legacy mode for ASGI.
|
|
Introducting application graceful stop. For now only used when application
process reach request limit value.
This closes #585 issue on GitHub.
|
|
|
|
|
|
Unit's ASGI implementation creates a new event loop to run an application for
each thread since 542b5b8c0647. This may cause unexpected exceptions or
strange bugs if asyncio synchronisation primitives are initialised before the
application starts (e.g. globally).
Although the approach with a new event loop for the main thread is consistent
and helps to prepare the application to run in multiple threads, it can be a
source of pain for people who just want to run single-threaded ASGI
applications in Unit.
This is related to #560 issue on GitHub.
|
|
An ASGI application can cancel the Future object returned by the receive()
call. In this case, Unit's ASGI implementation should not call set_result()
because the Future is already handled. In particular, the Starlette framework
was noted to cancel the received Future.
This patch adds a done() check for the Future before attempting a set_result().
This is related to #564 issue on GitHub.
|
|
The receive() call never blocks for a GET request and always returns the same
empty body message. The Starlette framework creates a separate task when
receive() is called in a loop until an 'http.disconnect' message is received.
The 'http.disconnect' message was previously issued after the response header
had been sent. However, the correct behavior is to respond with
'http.disconnect' after sending the response is complete.
This closes #564 issue on GitHub.
|
|
|
|
This partially reverts the optimisation introduced in 1d84b9e4b459 to avoid an
unpredictable block in nxt_unit_process_port_msg(). Under high load, this
function may never return control to its caller, and the external event loop
(in Node.js and Python asyncio) won't be able to process other scheduled
events.
To reproduce the issue, two request processing types are needed: 'fast' and
'furious'. The 'fast' one simply returns a small response, while the 'furious'
schedules asynchronous calls to external resources. Thus, if Unit is subjected
to a large amount of 'fast' requests, the 'furious' request processing freezes
until the high load ends.
The issue was found by Wu Jian Ping (@wujjpp) during Node.js stream
implementation discussion and relates to PR #502 on GitHub.
|
|
|
|
The WSGI environment dictionary contains a number of static items, that are
pre-initialized on application start. Then it's copied for each request to be
filled with request-related data.
Now this dictionary copy operation will be done between processing of requests,
which should save some CPU cycles during request processing and thus reduce
response latency for non-peak load periods.
|
|
|
|
|
|
Introducing manual protocol selection for 'universal' apps and frameworks.
|
|
|
|
This closes #459 issue on GitHub.
|
|
|
|
PyUnicode_GET_SIZE() in deprecated since 3.3 and will be removed in 3.12.
In version 3.9 it was explicitly marked by deprecation warning causing
compilation error with Unit.
PyUnicode_GET_LENGTH() must be used instead.
|
|
This closes #461 issue on GitHub.
|
|
Now it is possible to specify the name of the application callable using
optional parameter 'callable'. Default value is 'application'.
This closes #290 issue on GitHub.
|
|
The coming ASGI support requires raw HTTP headers format. Headers grouping
and upcase code were moved to WSGI module.
|
|
This is required for futher ASGI implementation.
|
|
No functional changes. Get ready for an increase in file number.
|