Thanks for update!

LGTM.

Sergos

On 3 Feb 2023, at 11:32, Sergey Kaplun <skaplun@tarantool.org> 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 <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

<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