Thanks for the patch! I've been running it for about an hour and
haven't seen the preallocated memory alert since, so it's looking good
here.

On Fri, 24 Mar 2023 at 03:07, Maxim Dounin <mdounin@mdounin.ru> wrote:
>
> Hello!
>
> On Thu, Mar 23, 2023 at 09:33:19PM +0100, Richard Stanway via nginx wrote:
>
> > Yes, when using the latest zlib-ng on nginx-1.21.6 I received the
> > alerts. Previous versions of zlib-ng have worked great after the 2021
> > patch. I tried to update it myself as follows based on advice of
> > zlib-ng GitHub issues, while it reduced the number of alerts logged it
> > did not completely solve the issue so it seems the memory requirements
> > may have further changed. While I would appreciate a proper patch
> > making it into nginx, the seemingly-frequent upstream changes may make
> > this difficult to maintain.
> >
> > - ctx->allocated = 8192 + 16 + (1 << (wbits + 2))
> > + ctx->allocated = 8192 + 288 + 16 + (1 << (wbits + 2))
> > + 131072 + (1 << (memlevel + 8));
>
> It looks like there are at least two changes in zlib-ng since I
> looked into it:
>
> - Window bits are no longer forced to 13 on compression level 1.
>
> - All allocations use custom alloc_aligned() wrapper, and
> therefore all allocations are larger than expected by (64 +
> sizeof(void*)).
>
> Further, due to the wrapper nginx sees all allocations as an
> allocation of 1 element of a given size, so it misinterprets
> some allocations as the state allocation.
>
> For example, allocations for a 1k responses are as follows (note
> "a:8192" in most of the lines, that is, nginx thinks these are
> state allocations):
>
> 2023/03/24 03:26:10 [debug] 36809#100069: *2 http gzip filter
> 2023/03/24 03:26:10 [debug] 36809#100069: *2 malloc: 21DEE5C0:176144
> 2023/03/24 03:26:10 [debug] 36809#100069: *2 gzip alloc: n:1 s:6036 a:8192 p:21DEE5C0
> 2023/03/24 03:26:10 [debug] 36809#100069: *2 gzip alloc: n:1 s:4180 a:8192 p:21DF05C0
> 2023/03/24 03:26:10 [debug] 36809#100069: *2 gzip alloc: n:1 s:4164 a:8192 p:21DF25C0
> 2023/03/24 03:26:10 [debug] 36809#100069: *2 gzip alloc: n:1 s:131140 a:131140 p:21DF45C0
> 2023/03/24 03:26:10 [debug] 36809#100069: *2 gzip alloc: n:1 s:4164 a:8192 p:21E14604
> 2023/03/24 03:26:10 [debug] 36809#100069: *2 gzip in: 21C31D84
>
> Allocations for 4k response are as follows (and generate an
> alert):
>
> 2023/03/24 03:44:29 [debug] 36863#100652: *2 http gzip filter
> 2023/03/24 03:44:29 [debug] 36863#100652: *2 malloc: 21DEE5C0:188432
> 2023/03/24 03:44:29 [debug] 36863#100652: *2 gzip alloc: n:1 s:6036 a:8192 p:21DEE5C0
> 2023/03/24 03:44:29 [debug] 36863#100652: *2 gzip alloc: n:1 s:16468 a:16468 p:21DF05C0
> 2023/03/24 03:44:29 [debug] 36863#100652: *2 gzip alloc: n:1 s:16452 a:16452 p:21DF4614
> 2023/03/24 03:44:29 [debug] 36863#100652: *2 gzip alloc: n:1 s:131140 a:131140 p:21DF8658
> 2023/03/24 03:44:29 [alert] 36863#100652: *2 gzip filter failed to use preallocated memory: 16452 of 16180 while sending response to client, client: 127.0.0.1, server: one, request: "GET /t/4k HTTP/1.1", host: "127.0.0.1:8080"
> 2023/03/24 03:44:29 [debug] 36863#100652: *2 malloc: 21DC58C0:16452
> 2023/03/24 03:44:29 [debug] 36863#100652: *2 gzip in: 21C31D98
>
> The "+ 288" you are using should be enough to cover additional
> memory used for alignment, but it is not enough to account
> for misinterpretation when using gzip_comp_level above 1 (so nginx
> won't allocate additional memory assuming window bits will be
> adjusted to 13).
>
> Please try the following patch, it should help with recent versions:
>
> # HG changeset patch
> # User Maxim Dounin <mdounin@mdounin.ru>
> # Date 1679622670 -10800
> # Fri Mar 24 04:51:10 2023 +0300
> # Node ID 67a0999550c3622e51639acb8bde57d199826f7e
> # Parent d1cf09451ae84b930ce66fa6d63ae3f7eeeac5a5
> Gzip: compatibility with recent zlib-ng versions.
>
> It now uses custom alloc_aligned() wrapper for all allocations,
> therefore all allocations are larger than expected by (64 + sizeof(void*)).
> Further, they are seen as allocations of 1 element. Relevant calculations
> were adjusted to reflect this, and state allocation is now protected
> with a flag to avoid misinterpreting other allocations as the zlib
> deflate_state allocation.
>
> Further, it no longer forces window bits to 13 on compression level 1,
> so the comment was adjusted to reflect this.
>
> diff --git a/src/http/modules/ngx_http_gzip_filter_module.c b/src/http/modules/ngx_http_gzip_filter_module.c
> --- a/src/http/modules/ngx_http_gzip_filter_module.c
> +++ b/src/http/modules/ngx_http_gzip_filter_module.c
> @@ -57,6 +57,7 @@ typedef struct {
> unsigned nomem:1;
> unsigned buffering:1;
> unsigned zlib_ng:1;
> + unsigned state_allocated:1;
>
> size_t zin;
> size_t zout;
> @@ -514,9 +515,10 @@ ngx_http_gzip_filter_memory(ngx_http_req
> } else {
> /*
> * Another zlib variant, https://github.com/zlib-ng/zlib-ng.
> - * It forces window bits to 13 for fast compression level,
> - * uses 16-byte padding in one of window-sized buffers, and
> - * uses 128K hash.
> + * It used to force window bits to 13 for fast compression level,
> + * uses (64 + sizeof(void*)) additional space on all allocations
> + * for alignment, 16-byte padding in one of window-sized buffers,
> + * and 128K hash.
> */
>
> if (conf->level == 1) {
> @@ -524,7 +526,8 @@ ngx_http_gzip_filter_memory(ngx_http_req
> }
>
> ctx->allocated = 8192 + 16 + (1 << (wbits + 2))
> - + 131072 + (1 << (memlevel + 8));
> + + 131072 + (1 << (memlevel + 8))
> + + 4 * (64 + sizeof(void*));
> ctx->zlib_ng = 1;
> }
> }
> @@ -926,13 +929,16 @@ ngx_http_gzip_filter_alloc(void *opaque,
>
> alloc = items * size;
>
> - if (items == 1 && alloc % 512 != 0 && alloc < 8192) {
> -
> + if (items == 1 && alloc % 512 != 0 && alloc < 8192
> + && !ctx->state_allocated)
> + {
> /*
> * The zlib deflate_state allocation, it takes about 6K,
> * we allocate 8K. Other allocations are divisible by 512.
> */
>
> + ctx->state_allocated = 1;
> +
> alloc = 8192;
> }
>
>
> --
> Maxim Dounin
> http://mdounin.ru/
> _______________________________________________
> nginx mailing list
> nginx@nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx
_______________________________________________
nginx mailing list
nginx@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx