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: 2+ 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
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