Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide/show prompt feature does not handle errors thrown from print #8136

Closed
CuriousGeorgiy opened this issue Jan 11, 2023 · 0 comments · Fixed by #8128
Closed

Hide/show prompt feature does not handle errors thrown from print #8136

CuriousGeorgiy opened this issue Jan 11, 2023 · 0 comments · Fixed by #8128
Assignees
Labels
2.11 Target is 2.11 and all newer release/master branches bug Something isn't working logger Issues related to logging subsystem

Comments

@CuriousGeorgiy
Copy link
Member

CuriousGeorgiy commented Jan 11, 2023

Consider the following snippet:

require('fiber').create(function() while true do pcall(function() print(setmetatable({}, {__tostring = error})) end) require('fiber').sleep(1) end end)

This code constantly erases the prompt and, AFAIC, the memory allocated in console_hide_prompt is also leaked, since after_cb never gets called:

if M.before_cb ~= nil then
M.before_cb()
end
M.raw_print(...)
if M.after_cb ~= nil then
M.after_cb()
end

/**
* Show saved readline's output and free saved strings.
*/
static void
console_show_prompt(void)
{
if (!console_can_hide_show_prompt())
return;
rl_set_prompt(saved_prompt);
free(saved_prompt);
saved_prompt = NULL;
if (saved_line_buffer == NULL) {
rl_replace_line("", 0);
} else {
rl_replace_line(saved_line_buffer, saved_line_buffer_len);
}
free(saved_line_buffer);
saved_line_buffer = NULL;
saved_line_buffer_len = 0;
rl_point = saved_point;
saved_point = 0;
rl_redisplay();
}

@CuriousGeorgiy CuriousGeorgiy added bug Something isn't working logger Issues related to logging subsystem labels Jan 11, 2023
@CuriousGeorgiy CuriousGeorgiy self-assigned this Jan 11, 2023
CuriousGeorgiy added a commit to CuriousGeorgiy/tarantool that referenced this issue Jan 11, 2023
Both of the callbacks in the `print` wrapper are expected to be called, but
`print` may throw errors, e.g.,
`print(setmetatable({}, {__tostring = error})`, so we need to call it in a
protected environment and execute the 'after' callback even if `print`
throws.

Closes tarantool#8136

NO_DOC=bugfix
CuriousGeorgiy added a commit to CuriousGeorgiy/tarantool that referenced this issue Jan 17, 2023
Both of the callbacks in the `print` wrapper are expected to be called, but
`print` may throw errors, e.g.,
`print(setmetatable({}, {__tostring = error})`, so we need to call it in a
protected environment and execute the 'after' callback even if `print`
throws.

Closes tarantool#8136

NO_CHANGELOG=<tarantoolgh-7186 was not released yet>
NO_DOC=bugfix
@locker locker added the 2.11 Target is 2.11 and all newer release/master branches label Jan 18, 2023
locker pushed a commit that referenced this issue Jan 18, 2023
Both of the callbacks in the `print` wrapper are expected to be called, but
`print` may throw errors, e.g.,
`print(setmetatable({}, {__tostring = error})`, so we need to call it in a
protected environment and execute the 'after' callback even if `print`
throws.

Closes #8136

NO_CHANGELOG=<gh-7186 was not released yet>
NO_DOC=bugfix
Lord-KA pushed a commit to Lord-KA/tarantool that referenced this issue Feb 27, 2023
Both of the callbacks in the `print` wrapper are expected to be called, but
`print` may throw errors, e.g.,
`print(setmetatable({}, {__tostring = error})`, so we need to call it in a
protected environment and execute the 'after' callback even if `print`
throws.

Closes tarantool#8136

NO_CHANGELOG=<tarantoolgh-7186 was not released yet>
NO_DOC=bugfix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.11 Target is 2.11 and all newer release/master branches bug Something isn't working logger Issues related to logging subsystem
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants