[Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.
Sergey Kaplun
skaplun at tarantool.org
Fri Feb 3 11:32:22 MSK 2023
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 <skaplun at 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
<snipped>
> > 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
More information about the Tarantool-patches
mailing list