Thanks for update! LGTM. Sergos > On 3 Feb 2023, at 11:32, Sergey Kaplun wrote: > > Hi, Sergos! > > Thanks for the review! > > Changed the test as you propose: > > =================================================================== > diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua > index a39ab6d1..0861ce14 100644 > --- a/test/tarantool-tests/lj-463-os-date-oom.test.lua > +++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua > @@ -4,15 +4,13 @@ local tap = require('tap') > 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 a 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. > +-- Try all available locales to check the behaviour. > -- 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') > +-- locale (ru_RU.utf8, sv_SE.utf8, etc.) may lead to OOM. > +for locale in io.popen('locale -a'):read('*a'):gmatch('([^\n]*)\n?') do > + os.setlocale(locale) > + os.date('%p') > +end > > test:ok(true, 'os.date() finished without OOM') > =================================================================== > > Branch is force-pushed. > > On 03.02.23, sergos wrote: >> 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’. > > Lucky you:). > >> >> Regards, >> Sergos >> >> >>> On 1 Feb 2023, at 10:46, Sergey Kaplun wrote: >>> >>> From: Mike Pall >>> >>> 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 > > > >>> 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 >>> >> > > -- > Best regards, > Sergey Kaplun