Skip to content

Commit

Permalink
LibLine: Stop registering the Notifier as a child Object
Browse files Browse the repository at this point in the history
We're already keeping it alive via `m_notifier`.
This makes the event loop quitting logic simpler by making less
deferred calls and removes a race condition where the notifier would be
deleted before the second deferred_invoke() would be invoked, leading
to a nullptr dereference.
Fixes SerenityOS#7822.
  • Loading branch information
alimpfard committed Jun 6, 2021
1 parent 73ff571 commit d8c5eec
Showing 1 changed file with 57 additions and 61 deletions.
118 changes: 57 additions & 61 deletions Userland/Libraries/LibLine/Editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -588,11 +588,8 @@ void Editor::interrupted()
m_is_editing = false;
restore();
m_notifier->set_enabled(false);
deferred_invoke([this](auto&) {
remove_child(*m_notifier);
m_notifier = nullptr;
Core::EventLoop::current().quit(Retry);
});
m_notifier = nullptr;
Core::EventLoop::current().quit(Retry);
}

void Editor::resized()
Expand Down Expand Up @@ -626,82 +623,81 @@ void Editor::really_quit_event_loop()
m_returned_line = string;

m_notifier->set_enabled(false);
deferred_invoke([this](auto&) {
remove_child(*m_notifier);
m_notifier = nullptr;
Core::EventLoop::current().quit(Exit);
});
m_notifier = nullptr;
Core::EventLoop::current().quit(Exit);
}

auto Editor::get_line(const String& prompt) -> Result<String, Editor::Error>
{
initialize();
m_is_editing = true;

if (m_configuration.operation_mode == Configuration::NoEscapeSequences || m_configuration.operation_mode == Configuration::NonInteractive) {
// Do not use escape sequences, instead, use LibC's getline.
size_t size = 0;
char* line = nullptr;
// Show the prompt only on interactive mode (NoEscapeSequences in this case).
if (m_configuration.operation_mode != Configuration::NonInteractive)
fputs(prompt.characters(), stderr);
auto line_length = getline(&line, &size, stdin);
// getline() returns -1 and sets errno=0 on EOF.
if (line_length == -1) {
if (line)
OwnPtr<Core::EventLoop> event_loop;
do {
initialize();
m_is_editing = true;

if (m_configuration.operation_mode == Configuration::NoEscapeSequences || m_configuration.operation_mode == Configuration::NonInteractive) {
// Do not use escape sequences, instead, use LibC's getline.
size_t size = 0;
char* line = nullptr;
// Show the prompt only on interactive mode (NoEscapeSequences in this case).
if (m_configuration.operation_mode != Configuration::NonInteractive)
fputs(prompt.characters(), stderr);
auto line_length = getline(&line, &size, stdin);
// getline() returns -1 and sets errno=0 on EOF.
if (line_length == -1) {
if (line)
free(line);
if (errno == 0)
return Error::Eof;

return Error::ReadFailure;
}
restore();
if (line) {
String result { line, (size_t)line_length, Chomp };
free(line);
if (errno == 0)
return Error::Eof;
return result;
}

return Error::ReadFailure;
}
restore();
if (line) {
String result { line, (size_t)line_length, Chomp };
free(line);
return result;
}

return Error::ReadFailure;
}

auto old_cols = m_num_columns;
auto old_lines = m_num_lines;
get_terminal_size();

if (m_configuration.enable_bracketed_paste)
fprintf(stderr, "\x1b[?2004h");
auto old_cols = m_num_columns;
auto old_lines = m_num_lines;
get_terminal_size();

if (m_num_columns != old_cols || m_num_lines != old_lines)
m_refresh_needed = true;
if (m_configuration.enable_bracketed_paste)
fprintf(stderr, "\x1b[?2004h");

set_prompt(prompt);
reset();
strip_styles(true);
if (m_num_columns != old_cols || m_num_lines != old_lines)
m_refresh_needed = true;

auto prompt_lines = max(current_prompt_metrics().line_metrics.size(), 1ul) - 1;
for (size_t i = 0; i < prompt_lines; ++i)
putc('\n', stderr);
set_prompt(prompt);
reset();
strip_styles(true);

VT::move_relative(-prompt_lines, 0);
auto prompt_lines = max(current_prompt_metrics().line_metrics.size(), 1ul) - 1;
for (size_t i = 0; i < prompt_lines; ++i)
putc('\n', stderr);

set_origin();
VT::move_relative(-prompt_lines, 0);

m_history_cursor = m_history.size();
set_origin();

refresh_display();
m_history_cursor = m_history.size();

Core::EventLoop loop;
refresh_display();

m_notifier = Core::Notifier::construct(STDIN_FILENO, Core::Notifier::Read);
add_child(*m_notifier);
if (!event_loop)
event_loop = make<Core::EventLoop>();

m_notifier->on_ready_to_read = [&] { try_update_once(); };
if (!m_incomplete_data.is_empty())
deferred_invoke([&](auto&) { try_update_once(); });
if (!m_notifier) {
m_notifier = Core::Notifier::construct(STDIN_FILENO, Core::Notifier::Read);
}

if (loop.exec() == Retry)
return get_line(prompt);
m_notifier->on_ready_to_read = [&] { try_update_once(); };
if (!m_incomplete_data.is_empty())
deferred_invoke([&](auto&) { try_update_once(); });
} while (event_loop->exec() == Retry);

return m_input_error.has_value() ? Result<String, Editor::Error> { m_input_error.value() } : Result<String, Editor::Error> { m_returned_line };
}
Expand Down

0 comments on commit d8c5eec

Please sign in to comment.