From e2a09c7742d2b74e3896ef99d3941ab1e46d2a15 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Thu, 13 Apr 2023 19:42:04 +0100 Subject: Convert 0-sized arrays to true flexible array members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declaring a 0-sized array (e.g 'char arr[0];') as the last member of a structure is a GNU extension that was used to implement flexible array members (FAMs) before they were standardised in C99 as simply '[]'. The GNU extension itself was introduced to work around a hack of declaring 1-sized arrays to mean a variable-length object. The advantage of the 0-sized (and true FAMs) is that they don't count towards the size of the structure. Unit already declares some true FAMs, but it also declared some 0-sized arrays. Converting these 0-sized arrays to true FAMs is not only good for consistency but will also allow better compiler checks now (as in a C99 FAM *must* be the last member of a structure and the compiler will warn otherwise) and in the future as doing this fixes a bunch of warnings (treated as errors in Unit by default) when compiled with -O2 -Warray-bounds -Wstrict-flex-arrays -fstrict-flex-arrays=3 (Note -Warray-bounds is enabled by -Wall and -Wstrict-flex-arrays seems to also be enabled via -Wall -Wextra, the -02 is required to make -fstrict-flex-arrays more effective, =3 is the default on at least GCC 14) such as CC build/src/nxt_upstream.o src/nxt_upstream.c: In function ‘nxt_upstreams_create’: src/nxt_upstream.c:56:18: error: array subscript i is outside array bounds of ‘nxt_upstream_t[0]’ {aka ‘struct nxt_upstream_s[]’} [-Werror=array-bounds=] 56 | string = nxt_str_dup(mp, &upstreams->upstream[i].name, &name); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from src/nxt_upstream.c:9: src/nxt_upstream.h:55:48: note: while referencing ‘upstream’ 55 | nxt_upstream_t upstream[0]; | ^~~~~~~~ Making our flexible array members proper C99 FAMs and ensuring any >0 sized trailing arrays in structures are really normal arrays will allow to enable various compiler options (such as the above and more) that will help keep our array usage safe. Changing 0-sized arrays to FAMs should have no effect on structure layouts/sizes (they both have a size of 0, although doing a sizeof() on a FAM will result in a compiler error). Looking at pahole(1) output for the nxt_http_route_ruleset_t structure for the [0] and [] cases... $ pahole -C nxt_http_route_ruleset_t /tmp/build/src/nxt_http_route.o typedef struct { uint32_t items; /* 0 4 */ /* XXX 4 bytes hole, try to pack */ nxt_http_route_rule_t * rule[]; /* 8 0 */ /* size: 8, cachelines: 1, members: 2 */ /* sum members: 4, holes: 1, sum holes: 4 */ /* last cacheline: 8 bytes */ } nxt_http_route_ruleset_t; $ pahole -C nxt_http_route_ruleset_t build/src/nxt_http_route.o typedef struct { uint32_t items; /* 0 4 */ /* XXX 4 bytes hole, try to pack */ nxt_http_route_rule_t * rule[]; /* 8 0 */ /* size: 8, cachelines: 1, members: 2 */ /* sum members: 4, holes: 1, sum holes: 4 */ /* last cacheline: 8 bytes */ } nxt_http_route_ruleset_t; Also checking with the size(1) command on the effected object files shows no changes to their sizes $ for file in build/src/nxt_upstream.o \ build/src/nxt_upstream_round_robin.o \ build/src/nxt_h1proto.o \ build/src/nxt_http_route.o \ build/src/nxt_http_proxy.o \ build/src/python/*.o; do \ size -G /tmp/${file} $file; echo; done text data bss total filename 640 418 0 1058 /tmp/build/src/nxt_upstream.o 640 418 0 1058 build/src/nxt_upstream.o text data bss total filename 929 351 0 1280 /tmp/build/src/nxt_upstream_round_robin.o 929 351 0 1280 build/src/nxt_upstream_round_robin.o text data bss total filename 11707 8281 16 20004 /tmp/build/src/nxt_h1proto.o 11707 8281 16 20004 build/src/nxt_h1proto.o text data bss total filename 8319 3101 0 11420 /tmp/build/src/nxt_http_route.o 8319 3101 0 11420 build/src/nxt_http_route.o text data bss total filename 1495 1056 0 2551 /tmp/build/src/nxt_http_proxy.o 1495 1056 0 2551 build/src/nxt_http_proxy.o text data bss total filename 4321 2895 0 7216 /tmp/build/src/python/nxt_python_asgi_http-python.o 4321 2895 0 7216 build/src/python/nxt_python_asgi_http-python.o text data bss total filename 4231 2266 0 6497 /tmp/build/src/python/nxt_python_asgi_lifespan-python.o 4231 2266 0 6497 build/src/python/nxt_python_asgi_lifespan-python.o text data bss total filename 12051 6090 8 18149 /tmp/build/src/python/nxt_python_asgi-python.o 12051 6090 8 18149 build/src/python/nxt_python_asgi-python.o text data bss total filename 28 1963 432 2423 /tmp/build/src/python/nxt_python_asgi_str-python.o 28 1963 432 2423 build/src/python/nxt_python_asgi_str-python.o text data bss total filename 5818 3518 0 9336 /tmp/build/src/python/nxt_python_asgi_websocket-python.o 5818 3518 0 9336 build/src/python/nxt_python_asgi_websocket-python.o text data bss total filename 4391 2089 168 6648 /tmp/build/src/python/nxt_python-python.o 4391 2089 168 6648 build/src/python/nxt_python-python.o text data bss total filename 9095 5909 152 15156 /tmp/build/src/python/nxt_python_wsgi-python.o 9095 5909 152 15156 build/src/python/nxt_python_wsgi-python.o Link: Link: Signed-off-by: Andrew Clayton --- src/python/nxt_python.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/python') diff --git a/src/python/nxt_python.h b/src/python/nxt_python.h index 37e6265e..f5154514 100644 --- a/src/python/nxt_python.h +++ b/src/python/nxt_python.h @@ -48,7 +48,7 @@ typedef struct { typedef struct { nxt_int_t count; - nxt_python_target_t target[0]; + nxt_python_target_t target[]; } nxt_python_targets_t; -- cgit From a9aa9e76db2766a681350c09947df848898531f6 Mon Sep 17 00:00:00 2001 From: Gourav Date: Wed, 26 Jun 2024 11:14:50 +0530 Subject: python: Support application factories Adds support for the app factory pattern to the Python language module. A factory is a callable that returns a WSGI or ASGI application object. Unit does not support passing arguments to factories. Setting the `factory` option to `true` instructs Unit to treat the configured `callable` as a factory. For example: "my-app": { "type": "python", "path": "/srv/www/", "module": "hello", "callable": "create_app", "factory": true } This is similar to other WSGI / ASGI servers. E.g., $ uvicorn --factory hello:create_app $ gunicorn 'hello:create_app()' The factory setting defaults to false. Closes: https://github.com/nginx/unit/issues/1106 Link: [ Commit message - Dan / Minor code tweaks - Andrew ] Signed-off-by: Andrew Clayton --- src/python/nxt_python.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) (limited to 'src/python') diff --git a/src/python/nxt_python.c b/src/python/nxt_python.c index 7c059649..aa0f65b1 100644 --- a/src/python/nxt_python.c +++ b/src/python/nxt_python.c @@ -403,11 +403,13 @@ nxt_python_set_target(nxt_task_t *task, nxt_python_target_t *target, char *callable, *module_name; PyObject *module, *obj; nxt_str_t str; + nxt_bool_t is_factory = 0; nxt_conf_value_t *value; static nxt_str_t module_str = nxt_string("module"); static nxt_str_t callable_str = nxt_string("callable"); static nxt_str_t prefix_str = nxt_string("prefix"); + static nxt_str_t factory_flag_str = nxt_string("factory"); module = obj = NULL; @@ -449,7 +451,30 @@ nxt_python_set_target(nxt_task_t *task, nxt_python_target_t *target, goto fail; } - if (nxt_slow_path(PyCallable_Check(obj) == 0)) { + value = nxt_conf_get_object_member(conf, &factory_flag_str, NULL); + if (value != NULL) { + is_factory = nxt_conf_get_boolean(value); + } + + if (is_factory) { + if (nxt_slow_path(PyCallable_Check(obj) == 0)) { + nxt_alert(task, + "factory \"%s\" in module \"%s\" " + "can not be called to fetch callable", + callable, module_name); + goto fail; + } + + obj = PyObject_CallObject(obj, NULL); + if (nxt_slow_path(PyCallable_Check(obj) == 0)) { + nxt_alert(task, + "factory \"%s\" in module \"%s\" " + "did not return callable object", + callable, module_name); + goto fail; + } + + } else if (nxt_slow_path(PyCallable_Check(obj) == 0)) { nxt_alert(task, "\"%s\" in module \"%s\" is not a callable object", callable, module_name); goto fail; -- cgit From ff6d504530ad2c126fc264744faa9e62bcc43fb9 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Sat, 22 Jun 2024 00:20:00 +0100 Subject: python: Constify some local static variables These somehow got missed in my previous constification patches... Signed-off-by: Andrew Clayton --- src/python/nxt_python.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/python') diff --git a/src/python/nxt_python.c b/src/python/nxt_python.c index aa0f65b1..7bbf3d49 100644 --- a/src/python/nxt_python.c +++ b/src/python/nxt_python.c @@ -406,10 +406,10 @@ nxt_python_set_target(nxt_task_t *task, nxt_python_target_t *target, nxt_bool_t is_factory = 0; nxt_conf_value_t *value; - static nxt_str_t module_str = nxt_string("module"); - static nxt_str_t callable_str = nxt_string("callable"); - static nxt_str_t prefix_str = nxt_string("prefix"); - static nxt_str_t factory_flag_str = nxt_string("factory"); + static const nxt_str_t module_str = nxt_string("module"); + static const nxt_str_t callable_str = nxt_string("callable"); + static const nxt_str_t prefix_str = nxt_string("prefix"); + static const nxt_str_t factory_flag_str = nxt_string("factory"); module = obj = NULL; -- cgit From 50b1aca3b8318c58f7073fe11911f1f0d52c651d Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Thu, 12 Sep 2024 16:48:10 +0100 Subject: python: Don't decrement a reference to a borrowed object On some Python 3.11 systems, 3.11.9 & 3.11.10, we were seeing a crash triggered by Py_Finalize() in nxt_python_atexit() when running one of our pytests, namely test/test_python_factory.py::test_python_factory_invalid_callable_value 2024/09/12 15:07:29 [alert] 5452#5452 factory "wsgi_invalid_callable" in module "wsgi" can not be called to fetch callable Fatal Python error: none_dealloc: deallocating None: bug likely caused by a refcount error in a C extension Python runtime state: finalizing (tstate=0x00007f560b88a718) Current thread 0x00007f560bde7ad0 (most recent call first): 2024/09/12 15:07:29 [alert] 5451#5451 app process 5452 exited on signal 6 (core dumped) This was due to obj = PyDict_GetItemString(PyModule_GetDict(module), callable); in nxt_python_set_target() which returns a *borrowed* reference, then due to the test meaning this is a `None` object we `goto fail` and call Py_DECREF(obj); which then causes `Py_Finalize()` to blow up. The simple fix is to just increment its reference count before the `goto fail`. Note: This problem only showed up under (the various versions of Python we test on); 3.11.9 & 3.11.10. It doesn't show up under; 3.6, 3.7, 3.9, 3.10, 3.12 Cc: Konstantin Pavlov Closes: https://github.com/nginx/unit/issues/1413 Fixes: a9aa9e76d ("python: Support application factories") Signed-off-by: Andrew Clayton --- src/python/nxt_python.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src/python') diff --git a/src/python/nxt_python.c b/src/python/nxt_python.c index 7bbf3d49..143d8d5d 100644 --- a/src/python/nxt_python.c +++ b/src/python/nxt_python.c @@ -462,6 +462,7 @@ nxt_python_set_target(nxt_task_t *task, nxt_python_target_t *target, "factory \"%s\" in module \"%s\" " "can not be called to fetch callable", callable, module_name); + Py_INCREF(obj); /* borrowed reference */ goto fail; } -- cgit