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

[sergos]

Thanks for update!

LGTM.

Sergos

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20230203/7554f237/attachment.htm>