technocloud-public/nginx-unit
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity
2,201 Commits 1 Branch 0 Tags
b7f1d7253a8f44f31c2e1a8d9c8962ef30be83e9
Commit Graph
28 Commits
This Branch
This Branch
All Branches
Author SHA1 Message Date
Andrew Clayton b7f1d7253a Isolation: Rename NXT_HAVE_CLONE -> NXT_HAVE_LINUX_NS. 
Due to the need to replace our use of clone/__NR_clone on Linux with
fork(2)/unshare(2) for enabling Linux namespaces(7) to keep the
pthreads(7) API working.  Let's rename NXT_HAVE_CLONE to
NXT_HAVE_LINUX_NS, i.e name it after the feature, not how it's
implemented, then in future if we change how we do namespaces again we
don't have to rename this.

Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-11-18 23:42:44 +00:00
Andrew Clayton 0277d8f103 Isolation: Fix the enablement of PR_SET_NO_NEW_PRIVS. 
This prctl(2) option is checked for in auto/isolation, unfortunately due
to a typo this feature has never been enabled.

In the auto/isolation script the feature name was down as
NXT_HAVE_PR_SET_NO_NEW_PRIVS0, which means we end up with the following
in build/nxt_auto_config.h

  #ifndef NXT_HAVE_PR_SET_NO_NEW_PRIVS0
  #define NXT_HAVE_PR_SET_NO_NEW_PRIVS0  1
  #endif

Whereas everywhere else is checking for NXT_HAVE_PR_SET_NO_NEW_PRIVS.

This also guards the inclusion of sys/prctl.h in src/nxt_process.c which
is required by a subsequent commit.

Fixes: e2b53e1 ("Added "rootfs" feature.")
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-11-25 10:32:20 +00:00
Andrew Clayton b2d1915578 Python: ASGI: Don't log asyncio.get_running_loop() errors. 
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>
 2023-02-07 13:11:10 +00:00
Andrew Clayton 843ae1b842 Python: ASGI: Switch away from asyncio.get_event_loop(). 
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 8dcb0b9987
  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 f27fbd9b4d
  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>
 2023-01-20 03:33:37 +00:00
Andrew Clayton 21bc8ee15f Python: ASGI: Factor out event loop creation to its own function. 
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>
 2023-01-20 03:27:59 +00:00
Andrew Clayton 420ddd4e49 PHP: Fix a potential problem parsing the path. 
@dward on GitHub reported an issue with a URL like

  http://foo.bar/test.php?blah=test.php/foo

where we would end up trying to run the script

  test.php?blah=test.php

In the PHP module the format 'file.php/' is treated as a special case in
nxt_php_dynamic_request() where we check the _path_ part of the url for
the string '.php/'.

The problem is that the path actually also contains the query string,
thus we were finding 'test.php/' in the above URL and treating that
whole path as the script to run.

The fix is simple, replace the strstr(3) with a memmem(3), where we can
limit the amount of path we use for the check.

The trick here and what is not obvious from the code is that while
path.start points to the whole path including the query string,
path.length only contains the length of the _path_ part.

NOTE: memmem(3) is a GNU extension and is neither specified by POSIX or
ISO C, however it is available on a number of other systems, including:
FreeBSD, OpenBSD, NetBSD, illumos, and macOS.

If it comes to it we can implement a simple alternative for systems
which lack memmem(3).

This also adds a test case (provided by @dward) to cover this.

Closes: <https://github.com/nginx/unit/issues/781>
Cc: Andrei Zeliankou <zelenkov@nginx.com>
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Reviewed-by: Andrei Zeliankou <zelenkov@nginx.com> [test]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-11-07 00:06:43 +00:00
Andrew Clayton a8dde6a5dd Fix endianness detection in nxt_websocket_header_t. 
The nxt_websocket_header_t structure defines the layout of a websocket
frame header.  As the websocket frame is mapped directly onto this
structure its layout needs to match how it's coming off the network.

The network being big endian means on big endian systems the structure
layout can simply match that of the websocket frame header.  On little
endian systems we need to reverse the two bytes.

This was done via the BYTE_ORDER, BIG_ENDIAN and LITTLE_ENDIAN macros,
however these are not universal, e.g they are _not_ defined on illumos
(OpenSolaris / OpenIndiana) and so we get the following compiler errors

In file included from src/nxt_h1proto.c:12:0:
src/nxt_websocket_header.h:25:13: error: duplicate member 'opcode'
     uint8_t opcode:4;
             ^~~~~~
src/nxt_websocket_header.h:26:13: error: duplicate member 'rsv3'
     uint8_t rsv3:1;
             ^~~~
src/nxt_websocket_header.h:27:13: error: duplicate member 'rsv2'
     uint8_t rsv2:1;
             ^~~~
src/nxt_websocket_header.h:28:13: error: duplicate member 'rsv1'
     uint8_t rsv1:1;
             ^~~~
src/nxt_websocket_header.h:29:13: error: duplicate member 'fin'
     uint8_t fin:1;
             ^~~
src/nxt_websocket_header.h:31:13: error: duplicate member 'payload_len'
     uint8_t payload_len:7;
             ^~~~~~~~~~~
src/nxt_websocket_header.h:32:13: error: duplicate member 'mask'
     uint8_t mask:1;
             ^~~~

This commit fixes that by using the new NXT_HAVE_{BIG,LITTLE}_ENDIAN
macros introduced in the previous commit.

Closes: <https://github.com/nginx/unit/issues/297>
Fixes: e501c74 ("Introducing websocket support in router and libunit.")
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-12-02 17:58:20 +00:00
Andrew Clayton c175e47cfe Autodetect endianness. 
In configure we set NXT_HAVE_LITTLE_ENDIAN for i386, amd64 and x86_64.
However that misses at least AArch64 (arm64) where it's usually run in
little endian mode.

However none of that really matters as NXT_HAVE_LITTLE_ENDIAN isn't used
anywhere.  So why this patch?

The only place we need to explicitly know about endianness is the
nxt_websocket_header_t structure where we lay it out differently
depending on endianness.

This is currently done using BYTE_ORDER, LITTLE_ENDIAN and BIG_ENDIAN
macros.

However on at least illumos (OpenSolaris / OpenIndiana) those macros are
not defined and we get compiler errors due to duplicate structure
members.

So let's use our own NXT_HAVE_{BIG,LITTLE}_ENDIAN macros.  However it
would be better to detect endianness programmatically as some
architectures can run in either mode, e.g Linux used to run in big
endian on PowerPC but has since switched to little endian (to match
x86).

This commit adds an auto/endian script (using a slightly modified
version of the test program from nginx's auto script), that checks for
the endianness of the platform being built on.  E.g

  checking for endianness ... little endian

The next commit will switch the nxt_websocket_header_t structure over to
these new macros.

Link: <https://github.com/nginx/unit/pull/298>
Link: <https://developer.ibm.com/articles/l-power-little-endian-faq-trs/>
Tested-by: Alejandro Colomar <alx@nginx.com>
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-12-02 17:20:37 +00:00
Andrew Clayton 96891a308b Python: Fix enabling of UTF-8 in some situations. 
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>
 2023-01-05 22:04:32 +00:00
Andrew Clayton fcbb26e92a Python: Do some cleanup in nxt_python3_init_config(). 
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>
 2022-12-30 00:07:20 +00:00
Andrew Clayton f88371ff1d Configuration: made large_header_buffers a valid setting. 
This is an extension to the previous commit, which made
large_header_buffer_size a valid configuration setting.

This commit makes a related value, large_header_buffers, a valid
configuration setting.

While large_header_buffer_size effectively limits the maximum size of
any single header (although unit will try to pack multiple headers into
a buffer if they wholly fit).

large_header_buffers limits how many of these 'large' buffers are
available. It makes sense to also allow this to be user set.

large_header_buffers is already set by the configuration system in
nxt_router.c it just isn't set as a valid config option in
nxt_conf_validation.c

With this change users can set this option in their config if required
by the following

    "settings": {
        "http": {
            "large_header_buffers": 8
        }
    },

It retains its default value of 4 if this is not set.

NOTE: This is being released as undocumented and subject to change as it
      exposes internal workings of unit.

Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-12-13 13:36:30 +00:00
Andrew Clayton dad7ef9a12 Configuration: made large_header_buffer_size a valid setting. 
@JanMikes and @tagur87 on GitHub both reported issues with long URLs
that were exceeding the 8192 byte large_header_buffer_size setting,
which resulted in a HTTP 431 error (Request Header Fields Too Large).

This can be resolved in the code by updating the following line in
src/nxt_router.c::nxt_router_conf_create()

    skcf->large_header_buffer_size = 8192;

However, requiring users to modify unit and install custom versions is
less than ideal. We could increase the value, but to what?

This commit takes the option of allowing the user to set this option in
their config by making large_header_buffer_size a valid configuration
setting.

large_header_buffer_size is already set by the configuration system in
nxt_router.c it just isn't set as a valid config option in
nxt_conf_validation.c

With this change users can set this option in their config if required
by the following

    "settings": {
        "http": {
            "large_header_buffer_size": 16384
        }
    },

It retains its default value of 8192 bytes if this is not set.

With this commit, without the above setting or too low a value, with a
long URL you get a 431 error. With the above setting set to a large
enough value, the request is successful.

NOTE: This setting really determines the maximum size of any single
      header _value_. Also, unit will try and place multiple values
      into a buffer _if_ they fully fit.

NOTE: This is being released as undocumented and subject to change as it
      exposes internal workings of unit.

Closes: <https://github.com/nginx/unit/issues/521>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-12-13 13:36:30 +00:00
Andrew Clayton f67a01b88f Isolation: wired up cgroup support to the config system. 
This hooks the cgroup support up to the config system so it can actually
be used.

To make use of this in unit a new "cgroup" section has been added to the
isolation configuration.

e.g

  "applications": {
      "python": {
          "type": "python",
          "processes": 5,
          "path": "/opt/unit/unit-cgroup-test/",
          "module": "app",

          "isolation": {
              "cgroup": {
                  "path": "app/python"
              }
          }
      }
  }

Now there are two ways to specify the path, relative, like the above
(without a leading '/') and absolute (with a leading '/').

In the above case the "python" application is placed into its own cgroup
under CGROUP_ROOT/<main unit process cgroup>/app/python. Whereas if you
specified say

  "path": "/unit/app/python"

Then the python application would be placed under
CGROUP_ROOT/unit/app/python

The first option allows you to easily take advantage of any resource
limits that have already been configured for unit.

With the second method (absolute pathname) if you know of an already
existing cgroup where you'd like to place it, you can, e.g

  "path": "/system.slice/unit/python"

Where system.slice has already been created by systemd and may already
have some overall system limits applied which would also apply to unit.
Limits apply down the hierarchy and lower groups can't exceed the
previous group limits.

So what does this actually look like? Lets take the unit-calculator
application[0] and have each of its applications placed into their own
cgroup. If we give each application a new section like

  "isolation": {
      "cgroup": {
          "path": "/unit/unit-calculator/add"
      }
  }

changing the path for each one, we can visualise the result with the
systemd-cgls command, e.g

  │   └─session-5.scope (#4561)
  │     ├─  6667 sshd: andrew [priv]
  │     ├─  6684 sshd: andrew@pts/0
  │     ├─  6685 -bash
  │     ├─ 12632 unit: main v1.28.0 [/opt/unit/sbin/unitd --control 127.0.0.1:808>
  │     ├─ 12634 unit: controller
  │     ├─ 12635 unit: router
  │     ├─ 13550 systemd-cgls
  │     └─ 13551 less
  ├─unit (#4759)
  │ └─unit-calculator (#5037)
  │   ├─subtract (#5069)
  │   │ ├─ 12650 unit: "subtract" prototype
  │   │ └─ 12651 unit: "subtract" application
  │   ├─multiply (#5085)
  │   │ ├─ 12653 unit: "multiply" prototype
  │   │ └─ 12654 unit: "multiply" application
  │   ├─divide (#5101)
  │   │ ├─ 12671 unit: "divide" prototype
  │   │ └─ 12672 node divide.js
  │   ├─sqroot (#5117)
  │   │ ├─ 12679 unit: "sqroot" prototype
  │   │ └─ 12680 /home/andrew/src/unit-calculator/sqroot/sqroot
  │   └─add (#5053)
  │     ├─ 12648 unit: "add" prototype
  │     └─ 12649 unit: "add" application

We used an absolute path so the cgroups will be created relative to the
main cgroupfs mount, e.g /sys/fs/cgroup

We can see that the main unit processes are in the same cgroup as the
shell from where they were started, by default child process are placed
into the same cgroup as the parent.

Then we can see that each application has been placed into its own
cgroup under /sys/fs/cgroup

Taking another example of a simple 5 process python application, with

  "isolation": {
      "cgroup": {
          "path": "app/python"
      }
  }

Here we have specified a relative path and thus the python application
will be placed below the existing cgroup that contains the main unit
process. E.g

  │   │ │ ├─app-glib-cinnamon\x2dcustom\x2dlauncher\x2d3-43951.scope (#90951)
  │   │ │ │ ├─   988 unit: main v1.28.0 [/opt/unit/sbin/unitd --no-daemon]
  │   │ │ │ ├─   990 unit: controller
  │   │ │ │ ├─   991 unit: router
  │   │ │ │ ├─ 43951 xterm -bg rgb:20/20/20 -fg white -fa DejaVu Sans Mono
  │   │ │ │ ├─ 43956 bash
  │   │ │ │ ├─ 58828 sudo -i
  │   │ │ │ ├─ 58831 -bash
  │   │ │ │ └─app (#107351)
  │   │ │ │   └─python (#107367)
  │   │ │ │     ├─ 992 unit: "python" prototype
  │   │ │ │     ├─ 993 unit: "python" application
  │   │ │ │     ├─ 994 unit: "python" application
  │   │ │ │     ├─ 995 unit: "python" application
  │   │ │ │     ├─ 996 unit: "python" application
  │   │ │ │     └─ 997 unit: "python" application

[0]: <https://github.com/lcrilly/unit-calculator>

Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-12-10 14:00:20 +00:00
Andrew Clayton c9e433a13d Isolation: wired up cgroup to build system. 
This commit enables the building of the cgroup code. This is only built
when the cgroupv2 filesystem is found.

If cgroupv2 support is found then

  cgroupv2: .................. YES

will be printed by ./configure

Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-12-10 14:00:20 +00:00
Andrew Clayton 867a839f10 Isolation: wired up per-application cgroup support internally. 
This commit hooks into the cgroup infrastructure added in the previous
commit to create per-application cgroups.

It does this by adding each "prototype process" into its own cgroup,
then each child process inherits its parents cgroup.

If we fail to create a cgroup we simply fail the process. This behaviour
may get enhanced in the future.

This won't actually do anything yet. Subsequent commits will hook this
up to the build and config systems.

Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-12-10 14:00:20 +00:00
Andrew Clayton 7d177faf3b Isolation: added core cgroup infrastructure. 
Firstly, this is not to be confused with CLONE_NEWCGROUP which unit
already supports and is related to namespaces. To re-cap, namespaces
allow processes to have different views of various parts of the system
such as filesystem mounts, networking, hostname etc.

Whereas cgroup[0] is a Linux kernel facility for collecting a bunch of
processes together to perform some task on the group as a whole, for
example to implement resource limits.

There are two parts to cgroup, the core part of organising processes
into a hierarchy and the controllers which are responsible for enforcing
resource limits etc.

There are currently two versions of the cgroup sub-system, the original
cgroup and a version 2[1] introduced in 3.16 (August 2014) and marked
stable in 4.5 (March 2016).

This commit supports the cgroup V2 API and implements the ability to
place applications into their own cgroup on a per-application basis.
You can put them each into their own cgroup or you can group some
together. The ability to set resource limits can easily be added in
future.

The initial use case of this would be to aid in observability of unit
applications which becomes much easier if you can just monitor them on a
per cgroup basis.

One thing to note about cgroup, is that unlike namespaces which are
controlled via system calls such as clone(2) and unshare(2), cgroups are
setup and controlled through the cgroupfs pseudo-filesystem.

cgroup is Linux only and this support will only be enabled if configure
finds the cgroup2 filesystem mount, e.g

  cgroup2 on /sys/fs/cgroup type cgroup2 (rw,nosuid,nodev,noexec,relatime,seclabel,nsdelegate,memory_recursiveprot)

The cgroups are removed on shutdown or as required on reconfiguration.

This commit just adds the basic infrastructure for using cgroups within
unit. Subsequent commits will wire up this support.

It supports creating cgroups relative to the main cgroup root and also
below the cgroup of the main unit process.

[0]: <https://man7.org/linux/man-pages/man7/cgroups.7.html>
[1]: <https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html>

Cc: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-12-10 14:00:20 +00:00
Andrew Clayton 9466daf9bd Added simple wrappers for fopen(3) and fclose(3). 
Add simple wrapper functions for fopen(3) and fclose(3) that are
somewhat akin to the nxt_file_open() and nxt_file_close() wrappers that
log errors.

Suggested-by: Alejandro Colomar <alx@nginx.com>
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-12-10 14:00:20 +00:00
Andrew Clayton e70653c766 Fix compilation with GCC and -O0. 
Andrei reported an issue with building unit when using '-O0' with GCC
producing the following compiler errors

cc -c -pipe -fPIC -fvisibility=hidden -O -W -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wmissing-prototypes -Werror -g -O0  -I src -I build   \
                      \
                     \
-o build/src/nxt_unit.o \
-MMD -MF build/src/nxt_unit.dep -MT build/src/nxt_unit.o \
src/nxt_unit.c
src/nxt_unit.c: In function ‘nxt_unit_log’:
src/nxt_unit.c:6601:9: error: ‘msg’ may be used uninitialized [-Werror=maybe-uninitialized]
 6601 |     p = nxt_unit_snprint_prefix(p, end, pid, level);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/nxt_unit.c:6682:1: note: by argument 2 of type ‘const char *’ to ‘nxt_unit_snprint_prefix’ declared here
 6682 | nxt_unit_snprint_prefix(char *p, const char *end, pid_t pid, int level)
      | ^~~~~~~~~~~~~~~~~~~~~~~
src/nxt_unit.c:6582:22: note: ‘msg’ declared here
 6582 |     char             msg[NXT_MAX_ERROR_STR], *p, *end;
      |                      ^~~
src/nxt_unit.c: In function ‘nxt_unit_req_log’:
src/nxt_unit.c:6645:9: error: ‘msg’ may be used uninitialized [-Werror=maybe-uninitialized]
 6645 |     p = nxt_unit_snprint_prefix(p, end, pid, level);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/nxt_unit.c:6682:1: note: by argument 2 of type ‘const char *’ to ‘nxt_unit_snprint_prefix’ declared here
 6682 | nxt_unit_snprint_prefix(char *p, const char *end, pid_t pid, int level)
      | ^~~~~~~~~~~~~~~~~~~~~~~
src/nxt_unit.c:6625:35: note: ‘msg’ declared here
 6625 |     char                          msg[NXT_MAX_ERROR_STR], *p, *end;
      |                                   ^~~
cc1: all warnings being treated as errors

The above was reproduced with

  $ ./configure --cc-opt=-O0 && ./configure python && make -j4

This warning doesn't happen on clang (15.0.4) or GCC (8.3) and seems to
have been introduced in GCC 11.  The above is from GCC (12.2.1, Fedora
37).

The trigger of this GCC issue is actually part of a commit I introduced
a few months back to constify some function parameters and it seems the
consensus for how to resolve this problem is to simply remove the const
qualifier from the *end parameter to nxt_unit_snprint_prefix().

Reported-by: Andrei Zeliankou <zelenkov@nginx.com>
Link: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100417>
Link: <https://github.com/samtools/htslib/pull/1285>
Link: <https://gcc.gnu.org/gcc-11/changes.html>
Fixes: 4418f99 ("Constified numerous function parameters.")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-12-08 13:45:01 +00:00
Andrew Clayton 491d0f700f Python: Added support for Python 3.11. 
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>
 2022-12-06 02:51:03 +00:00
Andrew Clayton a03274456b PHP: allowed to specify URLs without a trailing '/'. 
Both @lucatacconi & @mwoodpatrick reported what appears to be the same
issue on GitHub. Namely that when using the PHP language module and
trying to access a URL that is a directory but without specifying the
trailing '/', they were getting a '503 Service Unavailable' error.

Note: This is when _not_ using the 'script' option.

E.g with the following config

  {
      "listeners": {
          "[::1]:8080": {
              "pass": "applications/php"
          }
      },

      "applications": {
          "php": {
              "type": "php",
              "root": "/var/tmp/unit-php"
          }
      }
  }

and with a directory path of /var/tmp/unit-php/foo containing an
index.php, you would see the following

  $ curl http://localhost/foo
  <title>Error 503</title>
  Error 503

However

  $ curl http://localhost/foo/

would work and serve up the index.php

This commit fixes the above so you get the desired behaviour without
specifying the trailing '/' by doing the following

  1] If the URL doesn't end in .php and doesn't have a trailing '/'
     then check if the requested path is a directory.

  2) If it is a directory then create a 301 re-direct pointing to it.
     This matches the behaviour of the likes of nginx, Apache and
     lighttpd.

     This also matches the behaviour of the "share" action in Unit.

This doesn't effect the behaviour of the 'script' option which bypasses
the nxt_php_dynamic_request() function.

This also adds a couple of tests to test/test_php_application.py to
ensure this continues to work.

Closes: <https://github.com/nginx/unit/issues/717>
Closes: <https://github.com/nginx/unit/issues/753>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-11-02 14:22:39 +00:00
Andrew Clayton 58248a6220 Fixed some function definitions. 
Future releases of GCC will render function definitions like

  func()

invalid by default. See the previous commit 09f88c9 ("Fixed main()
prototypes in auto tests.") for details.

Such functions should be defined like

  func(void)

This is a good thing to do regardless of the upcoming GCC changes.

Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-10-28 03:18:33 +01:00
Andrew Clayton 8f0dd9478e Fixed main() prototypes in auto tests. 
Future releases of GCC are planning to remove[0] default support for
some old features that were removed from C99 but GCC still accepts.

We can test for these changes by using the following -Werror=
directives

  -Werror=implicit-int
  -Werror=implicit-function-declaration
  -Werror=int-conversion
  -Werror=strict-prototypes
  -Werror=old-style-definition

Doing so revealed an issue with the auto/ tests in that the test
programs always define main as

  int main()

rather than

  int main(void)

which results in a bunch of errors like

build/autotest.c:3:23: error: function declaration isn't a prototype [-Werror=strict-prototypes]
    3 |                   int main() {
      |                       ^~~~
build/autotest.c: In function 'main':
build/autotest.c:3:23: error: old-style function definition [-Werror=old-style-definition]

The fix was easy, it only required fixing the main prototype with

  find -type f -exec sed -i 's/int main() {/int main(void) {/g' {} \;

Regardless of these upcoming GCC changes, this is probably a good thing
to do anyway for correctness.

[0]: https://fedoraproject.org/wiki/Changes/PortingToModernC

Link: <https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/CJXKTLXJUPZ4F2C2VQOTNMEA5JAUPMBD/>
Link: <https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/6SGHPHPAXKCVJ6PUZ57WVDQ5TDBVIRMF/>
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-10-28 03:17:31 +01:00
Andrew Clayton 7a928b33b4 Set git diff driver for C source code files. 
Git can be told to apply language-specific rules when generating diffs.
Enable this for C source code files (*.c and *.h) so that function names
are printed right. Specifically, doing so prevents "git diff" from
mistakenly considering unindented goto labels as function names.

This has the same effect as adding

    [diff "default"]
        xfuncname = "^[[:alpha:]$_].*[^:]$"

to your git config file.

e.g get

    @@ -10,7 +10,7 @@ int main(void)

instead of

    @@ -10,7 +10,7 @@ again:

This makes use of the gitattributes(5) infrastructure.

Link: <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=218dd85887da3d7d08119de18e9d325fcf30d7a4>
Link: <https://git.kernel.org/pub/scm/git/git.git/commit/?id=e82675a040d559c56be54255901138a979eeec21>
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-10-26 01:23:02 +01:00
Andrew Clayton f8b892e1fa Fixed the build on MacOS (and others). 
@alejandro-colomar reported that the build was broken on MacOS

cc  -o build/unitd -pipe -fPIC -fvisibility=hidden -O -W -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -fstrict-aliasing -Wstrict-overflow=5 -Wmissing-prototypes -Werror -g   \
		build/src/nxt_main.o  build/libnxt.a \
		    \
                      \
                    -L/usr/local/Cellar/pcre2/10.40/lib -lpcre2-8
Undefined symbols for architecture x86_64:
  "_nxt_fs_mkdir_parent", referenced from:
      _nxt_runtime_pid_file_create in libnxt.a(nxt_runtime.o)
      _nxt_runtime_controller_socket in libnxt.a(nxt_controller.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [build/unitd] Error 1

This was due to commit 57fc920 ("Socket: Created control socket & pid file
directories.").

This happened because this commit introduced the usage of
nxt_fs_mkdir_parent() in core code which uses nxt_fs_mkdir(), both of
these are defined in src/nxt_fs.c. It turns out however that this file
doesn't get built on MacOS (or any system that isn't Linux or that
lacks a FreeBSD compatible nmount(2) system call) due to the following

In auto/sources we have

  if [ $NXT_HAVE_ROOTFS = YES ]; then
      NXT_LIB_SRCS="$NXT_LIB_SRCS src/nxt_fs.c"
  fi

NXT_HAVE_ROOTFS is set in auto/isolation

  If [ $NXT_HAVE_MOUNT = YES -a $NXT_HAVE_UNMOUNT = YES ]; then
      NXT_HAVE_ROOTFS=YES

      cat << END >> $NXT_AUTO_CONFIG_H
  #ifndef NXT_HAVE_ISOLATION_ROOTFS
  #define NXT_HAVE_ISOLATION_ROOTFS  1
  #endif
  END

  fi

While we do have a check for a generic umount(2) which is found on
MacOS, for mount(2) we currently only check for the Linux mount(2) and
FreeBSD nmount(2) system calls. So NXT_HAVE_ROOTFS is set to NO on MacOS
and we don't build src/nxt_fs.c

This fixes the immediate build issue by taking the mount/umount OS
support out of nxt_fs.c into a new nxt_fs_mount.c file which is guarded
by the above while we now build nxt_fs.c unconditionally.

This should fix the build on any _supported_ system.

Reported-by: Alejandro Colomar <alx@nginx.com>
Fixes: 57fc920 ("Socket: Created control socket & pid file directories.")
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-10-14 12:25:47 +01:00
Andrew ClaytonandAndrew Clayton d806a90609 Ruby: used nxt_ruby_exception_log() in nxt_ruby_rack_init(). 
For consistency use nxt_ruby_exception_log() rather than nxt_alert() in
nxt_ruby_rack_init().

Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-10-04 19:33:11 +01:00
Andrew ClaytonandAndrew Clayton b00983369b Renamed a couple of members of nxt_unit_request_t. 
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>
 2022-10-03 14:32:28 +01:00
Andrew ClaytonandAndrew Clayton 57fc9201cb Socket: Created control socket & pid file directories. 
@alejandro-colomar reported an issue on GitHub whereby Unit would fail
to start due to not being able to create the control socket (a Unix
Domain Socket)

  2022/08/05 20:12:22 [alert] 21613#21613 bind(6,
  unix:/opt/local/unit/var/run/unit/control.unit.sock.tmp)
  failed (2: No such file or directory)

This could happen if the control socket was set to a directory that
doesn't exist. A common place to put the control socket would be under
/run/unit, and while /run will exist, /run/unit may well not (/run
is/should be cleared on each boot).

The pid file would also generally go under /run/unit, though this is
created after the control socket, however it could go someplace else so
we should also ensure its directory exists.

This commit will try to create the pid file and control sockets parent
directory. In some cases the user will need to ensure that the rest of
the path already exists.

This adds a new nxt_fs_mkdir_parent() function that given a full path
to a file (or directory), strips the last component off before passing
the remaining directory path to nxt_fs_mkdir().

Cc: Konstantin Pavlov <thresh@nginx.com>
Closes: <https://github.com/nginx/unit/issues/742>
Reported-by: Alejandro Colomar <alx@nginx.com>
Reviewed-by: Alejandro Colomar <alx@nginx.com>
Tested-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-10-03 14:26:45 +01:00
Andrew Clayton 2e69b7eb57 Added a .mailmap file. 
This file is used by git (maybe also hg) to map different identities.

It can be used to map different names/email addresses to a preferred
name/email. So if you have authored/committed under different
names/email addresses you can map all these to a particular name/email.
Certain git commands (log, shortlog, show, blame etc) will consult this
file and show the Author/Committer accordingly.

Note: This does _not_ change history, this is simply used by various
commands to alter their output and can be disabled for some commands
with the --no-mailmap option.

This can be useful for commands like git shortlog so that all your
commits are shown under a single identity and also so people have an
up to date email address should they wish to contact you. And just for
overall consistency.

Seeing as I've already committed under two different email addresses
(not counting this one), I've put entries in this file to map my email
addresses to my @nginx.com one.

See also, gitmailmap(5).

Acked-by: Alex Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
 2022-10-03 14:16:44 +01:00