Tarantool development patches archive
 help / color / mirror / Atom feed
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


             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