From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org,
Sergey Kaplun <skaplun@tarantool.org>
Subject: [Tarantool-patches] [PATCH luajit][v2] Always close profiler output file.
Date: Wed, 12 Feb 2025 15:20:22 +0300 [thread overview]
Message-ID: <80360fb2e570ce8d54ff59ccf0bcd5dfb6b98527.1739362814.git.sergeyb@tarantool.org> (raw)
From: Mike Pall <mike>
Reported by Guilherme Batalheiro.
(cherry picked from commit fca66335d131669cf017420af6963a7565babb58)
Before the patch, a function `prof_finish` wrote a string
`No samples collected` to a profiler output file and then exited.
Due to early exit, thethe output file handle stay open. This patch
fixes the condition and the file handle is closed even if the
number of samples is equal to 0.
Sergey Bronnikov:
* added the description and the test for the problem
Part of tarantool/tarantool#11055
---
Changes v2:
- Added fixes according to comments by Sergey Kaplun.
Branch: https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-close-file-profiler
Related issues:
- https://github.com/luajIT/luajIT/issues/1304
- https://github.com/tarantool/tarantool/issues/11055
src/jit/p.lua | 4 +--
...close-profile-dump-with-0-samples.test.lua | 31 +++++++++++++++++++
.../script.lua | 19 ++++++++++++
test/tarantool-tests/utils/tools.lua | 4 +++
4 files changed, 55 insertions(+), 3 deletions(-)
create mode 100644 test/tarantool-tests/lj-1304-close-profile-dump-with-0-samples.test.lua
create mode 100644 test/tarantool-tests/lj-1304-close-profile-dump-with-0-samples/script.lua
diff --git a/src/jit/p.lua b/src/jit/p.lua
index 4569d69e..89b49584 100644
--- a/src/jit/p.lua
+++ b/src/jit/p.lua
@@ -228,9 +228,7 @@ local function prof_finish()
local samples = prof_samples
if samples == 0 then
if prof_raw ~= true then out:write("[No samples collected]\n") end
- return
- end
- if prof_ann then
+ elseif prof_ann then
prof_annotate(prof_count1, samples)
else
prof_top(prof_count1, prof_count2, samples, "")
diff --git a/test/tarantool-tests/lj-1304-close-profile-dump-with-0-samples.test.lua b/test/tarantool-tests/lj-1304-close-profile-dump-with-0-samples.test.lua
new file mode 100644
index 00000000..614ecc69
--- /dev/null
+++ b/test/tarantool-tests/lj-1304-close-profile-dump-with-0-samples.test.lua
@@ -0,0 +1,31 @@
+local tap = require('tap')
+local test = tap.test('lj-1304-close-profile-dump-with-0-samples'):skipcond({
+ ['Test requires /proc filesystem'] = jit.os == 'OSX',
+})
+local utils = require('utils')
+
+test:plan(1)
+
+-- Test file to demonstrate LuaJIT incorrect behaviour with missed
+-- closing a file handle for the profile output file.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1304
+
+local p_postfix = 'sysprofdata'
+local p_filename = utils.tools.profilename(p_postfix)
+
+-- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
+-- with the given environment, launch options and CLI arguments.
+local script = utils.exec.makecmd(arg)
+-- Execute a Lua script with start and stop LuaJIT profiler,
+-- it is expected no samples found by profiler. The script's
+-- output is suppressed, it is not interested.
+local _ = script(p_filename, p_postfix)
+
+local p_content = io.open(p_filename):read('a*')
+test:is(utils.tools.trim(p_content), '[No samples collected]',
+ 'profile dump has no samples')
+
+-- Teardown.
+os.remove(p_filename)
+
+test:done(true)
diff --git a/test/tarantool-tests/lj-1304-close-profile-dump-with-0-samples/script.lua b/test/tarantool-tests/lj-1304-close-profile-dump-with-0-samples/script.lua
new file mode 100644
index 00000000..864958f5
--- /dev/null
+++ b/test/tarantool-tests/lj-1304-close-profile-dump-with-0-samples/script.lua
@@ -0,0 +1,19 @@
+local jit_p = require('jit.p')
+
+-- Using a bigger interval to make sure that there will be no
+-- samples collected.
+local p_options = 'i99999'
+local p_filename = assert(arg[1], 'filename argument is missing')
+local p_postfix = assert(arg[2], 'postfix argument is missing')
+
+jit_p.start(p_options, p_filename)
+
+-- No code to generate profiling samples.
+
+-- Stop profiler to execute `jit/p.lua:prof_fmt()`. With zero
+-- samples it triggers early return without closing the file.
+jit_p.stop()
+
+-- Make sure LuaJIT profiler is close a file handle.
+local ls_output = io.popen('ls -l /proc/$$/fd'):read('a*')
+assert(ls_output:find(p_postfix) == nil, 'file is open')
diff --git a/test/tarantool-tests/utils/tools.lua b/test/tarantool-tests/utils/tools.lua
index 33fcae78..9cb65daf 100644
--- a/test/tarantool-tests/utils/tools.lua
+++ b/test/tarantool-tests/utils/tools.lua
@@ -21,4 +21,8 @@ function M.read_file(path)
return content
end
+function M.trim(str)
+ return (str:gsub('^%s*(.-)%s*$', '%1'))
+end
+
return M
--
2.34.1
next reply other threads:[~2025-02-12 12:22 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-12 12:20 Sergey Bronnikov via Tarantool-patches [this message]
2025-02-13 15:31 ` Sergey Kaplun via Tarantool-patches
2025-02-25 8:58 ` Sergey Bronnikov via Tarantool-patches
2025-02-26 11:58 ` Sergey Kaplun via Tarantool-patches
2025-02-27 13:07 ` Sergey Bronnikov via Tarantool-patches
2025-03-04 15:40 ` Sergey Kaplun via Tarantool-patches
2025-03-05 14:13 ` Sergey Bronnikov via Tarantool-patches
2025-03-05 14:58 ` Sergey Kaplun via Tarantool-patches
2025-03-05 15:09 ` Sergey Bronnikov via Tarantool-patches
2025-03-12 11:12 ` Sergey Kaplun via Tarantool-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=80360fb2e570ce8d54ff59ccf0bcd5dfb6b98527.1739362814.git.sergeyb@tarantool.org \
--to=tarantool-patches@dev.tarantool.org \
--cc=estetus@gmail.com \
--cc=skaplun@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH luajit][v2] Always close profiler output file.' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox