Skip to content

Commit

Permalink
sysprof: fix a message with stop without run
Browse files Browse the repository at this point in the history
The function `misc.sysprof.stop()` reports that profiler is
already running:

| $ ./src/luajit -e 'print(misc.sysprof.stop())'
| nil     profiler is running already     22

The patch splits error code `PROFILE_ERRRUN` to a two separate
error codes `PROFILE_ERRISRUN` and `PROFILE_ERRNOTRUN` to handle
both in `sysprof_error()` and fixes aforementioned problem.

Follows up tarantool/tarantool#781
  • Loading branch information
ligurio committed Feb 11, 2025
1 parent f5ee834 commit 8e82ff7
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 10 deletions.
11 changes: 8 additions & 3 deletions src/lib_misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ static int sysprof_error(lua_State *L, int status, const char *err_details)
lua_pushinteger(L, EINVAL);
return 3;
#if LJ_HASSYSPROF
case PROFILE_ERRRUN:
case PROFILE_ERRISRUN:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_ISRUNNING));
if (err_details) {
Expand All @@ -298,6 +298,11 @@ static int sysprof_error(lua_State *L, int status, const char *err_details)
}
lua_pushinteger(L, EINVAL);
return 3;
case PROFILE_ERRNOTRUN:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_NOTRUNNING));
lua_pushinteger(L, EINVAL);
return 3;
case PROFILE_ERRIO:
return luaL_fileresult(L, 0, NULL);
#endif
Expand Down Expand Up @@ -415,7 +420,7 @@ LJLIB_CF(misc_memprof_start)
lua_pushinteger(L, EINVAL);
return 3;
#if LJ_HASMEMPROF
case PROFILE_ERRRUN:
case PROFILE_ERRISRUN:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_ISRUNNING));
lua_pushinteger(L, EINVAL);
Expand Down Expand Up @@ -444,7 +449,7 @@ LJLIB_CF(misc_memprof_stop)
lua_pushinteger(L, EINVAL);
return 3;
#if LJ_HASMEMPROF
case PROFILE_ERRRUN:
case PROFILE_ERRNOTRUN:
lua_pushnil(L);
lua_pushstring(L, err2msg(LJ_ERR_PROF_NOTRUNNING));
lua_pushinteger(L, EINVAL);
Expand Down
4 changes: 2 additions & 2 deletions src/lj_memprof.c
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ int lj_memprof_start(struct lua_State *L, const struct lj_memprof_options *opt)
if (mp->state != MPS_IDLE) {
/* Clean up resources. Ignore possible errors. */
opt->on_stop(opt->ctx, opt->buf);
return PROFILE_ERRRUN;
return PROFILE_ERRISRUN;
}

/* Discard possible old errno. */
Expand Down Expand Up @@ -319,7 +319,7 @@ int lj_memprof_stop(struct lua_State *L)
}

if (mp->state != MPS_PROFILE)
return PROFILE_ERRRUN;
return PROFILE_ERRNOTRUN;

if (mp->g != G(L))
return PROFILE_ERRUSE;
Expand Down
4 changes: 2 additions & 2 deletions src/lj_sysprof.c
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ static int sysprof_validate(struct sysprof *sp,

case SPS_PROFILE:
case SPS_HALT:
return PROFILE_ERRRUN;
return PROFILE_ERRISRUN;

default:
lj_assertX(0, "bad sysprof profiler state");
Expand Down Expand Up @@ -494,7 +494,7 @@ int lj_sysprof_stop(lua_State *L)
struct lj_wbuf *out = &sp->out;

if (SPS_IDLE == sp->state)
return PROFILE_ERRRUN;
return PROFILE_ERRNOTRUN;
else if (G(L) != g)
return PROFILE_ERRUSE;

Expand Down
3 changes: 2 additions & 1 deletion src/lmisclib.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,10 @@ struct luam_Sysprof_Options {

#define PROFILE_SUCCESS 0
#define PROFILE_ERRUSE 1
#define PROFILE_ERRRUN 2
#define PROFILE_ERRISRUN 2
#define PROFILE_ERRMEM 3
#define PROFILE_ERRIO 4
#define PROFILE_ERRNOTRUN 5

LUAMISC_API int luaM_sysprof_set_writer(luam_Sysprof_writer writer);

Expand Down
5 changes: 3 additions & 2 deletions test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ local test = tap.test("misclib-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
Expand Down Expand Up @@ -98,7 +98,8 @@ assert(res, err)

-- Not running.
res, err, errno = misc.sysprof.stop()
test:ok(res == nil and err, "result status and error with not running")
test:is(res, nil, "result status with not running")
test:ok(err:match("profiler is not running"), "error with not running")
test:ok(type(errno) == "number", "errno with not running")

-- Bad path.
Expand Down

0 comments on commit 8e82ff7

Please sign in to comment.