Re: [Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.

[sergos via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi!

Thanks for the patch!

I can propose to update test with something like

  for i in io.popen('locale -a', 'r'):read('*a'):gmatch("([^\n]*)\n?") do
      os.setlocale(i)
      os.date(%p)
  end

to run the test across all available options. The reason is I got:

  tarantool> os.setlocale('ru_RU.utf8')
  ---
  - null

on my system, so I don’t by the ‘high probability’. 

Regards,
Sergos


> On 1 Feb 2023, at 10:46, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> From: Mike Pall <mike>
> 
> Thanks to Jesper Lundgren.
> 
> (cherry picked from commit fc63c938b522e147ea728b75f385728bf4a8fc35)
> 
> When `strftime()` returns empty result (for example, for "%p" on some
> locales or when LuaJIT is built with musl's `strftime()` and format
> string is invalid), the `os.date()` endless retries to format result
> and reallocates buffer for resulting string. This leads to OOM.
> 
> This patch limits amount of retries by 4.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#8069
> ---
> 
> PR: https://github.com/tarantool/tarantool/pull/8237
> Issues:
> * https://github.com/LuaJIT/LuaJIT/issues/463
> * https://github.com/tarantool/tarantool/issues/8069
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci
> 
> src/lib_os.c                                  |  4 ++--
> .../lj-463-os-date-oom.test.lua               | 19 +++++++++++++++++++
> 2 files changed, 21 insertions(+), 2 deletions(-)
> create mode 100644 test/tarantool-tests/lj-463-os-date-oom.test.lua
> 
> diff --git a/src/lib_os.c b/src/lib_os.c
> index 9e78d49a..ffbc3fdc 100644
> --- a/src/lib_os.c
> +++ b/src/lib_os.c
> @@ -205,12 +205,12 @@ LJLIB_CF(os_date)
>     setboolfield(L, "isdst", stm->tm_isdst);
>   } else if (*s) {
>     SBuf *sb = &G(L)->tmpbuf;
> -    MSize sz = 0;
> +    MSize sz = 0, retry = 4;
>     const char *q;
>     for (q = s; *q; q++)
>       sz += (*q == '%') ? 30 : 1;  /* Overflow doesn't matter. */
>     setsbufL(sb, L);
> -    for (;;) {
> +    while (retry--) {  /* Limit growth for invalid format or empty result. */
>       char *buf = lj_buf_need(sb, sz);
>       size_t len = strftime(buf, sbufsz(sb), s, stm);
>       if (len) {
> diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua
> new file mode 100644
> index 00000000..cce78b6e
> --- /dev/null
> +++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua
> @@ -0,0 +1,19 @@
> +local tap = require('tap')
> +
> +-- See also https://github.com/LuaJIT/LuaJIT/issues/463.
> +local test = tap.test('lj-463-os-date-oom')
> +test:plan(1)
> +
> +-- The ru_RU.utf8 locale is chosen as one that will be set on a
> +-- developer's PC with high possibility. It may be unavailable at
> +-- CI, so don't check the status and result of function calls.
> +-- If it's unavailable `os.setlocale()` does nothing.
> +-- Before the patch, the call to `os.date('%p')` on non-standard
> +-- locale may lead to OOM.
> +
> +os.setlocale('ru_RU.utf8')
> +os.date('%p')
> +
> +test:ok(true, 'os.date() finished without OOM')
> +
> +os.exit(test:check() and 0 or 1)
> -- 
> 2.34.1
>