Hi, Sergey!

thanks for valuable comments!

Sergey

Hi, Sergey!
Thanks for the patch!
Please consider my comments below.

On 04.02.25, Sergey Bronnikov wrote:

The function `misc.sysprof.stop()` reports that profiler is
already running:

| $ ./build/src/luajit -e 'print(misc.sysprof.stop())'

Minor: You may omit a ./build here, but this is a matter of taste, so
feel free to ignore.

| nil     profiler is running already     22

The patch fixes that.

Unfortunatelly not:

| $ git --no-pager log --oneline -n1 --no-decorate && ./src/luajit -e 'print(misc.sysprof.stop())'
| b49c5fac sysprof: fix a message with stop without run
| nil     profiler is running already     22

See my comments below.

Please add the follow-up to the tarantool/tarantool#781.

---
 src/lj_sysprof.c                                             | 2 +-
 test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/lj_sysprof.c b/src/lj_sysprof.c
index 88c7a41b..b76f503c 100644
--- a/src/lj_sysprof.c
+++ b/src/lj_sysprof.c
@@ -493,7 +493,7 @@ int lj_sysprof_stop(lua_State *L)
   global_State *g = sp->g;
   struct lj_wbuf *out = &sp->out;
 
-  if (SPS_IDLE == sp->state)
+  if (SPS_PROFILE != sp->state)

This check is correct. It checks that the profiler has no error or is
not running (i.e., that it is stopped). With the new check, a case with
an error during profiling will be masked.

It looks like we should add this case in the `misc.sysprof.stop()` (it
uses `sysprof_error()` which handles start cases).

     return PROFILE_ERRRUN;
   else if (G(L) != g)
     return PROFILE_ERRUSE;
diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
index 68a4b72f..91f9ca5c 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
@@ -10,7 +10,7 @@ local test = tap.test("misc-sysprof-lapi"):skipcond({
   ["Disabled due to #10803"] = os.getenv("LUAJIT_TEST_USE_VALGRIND"),
 })
 
-test:plan(33)
+test:plan(34)
 
 jit.off()
 -- XXX: Run JIT tuning functions in a safe frame to avoid errors
@@ -98,7 +98,8 @@ assert(res, err)
 
 -- Not running.
 res, err, errno = misc.sysprof.stop()
-test:ok(res == nil and err, "res and error with not running")
+test:is(res, nil, "res with not running")
+test:ok(err:match("profiler is running already"), "error with not running")

I suppose there is bad copy-pasting here. It should be: 'profiler is not
running'. This is why the test passes.

 test:ok(type(errno) == "number", "errno with not running")
 
 -- Bad path.
-- 
2.34.1