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.luaindex 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')+endtest: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