From e2ae5a78df00875e47f4487b23c845333cad1997 Mon Sep 17 00:00:00 2001 From: eidheim Date: Thu, 30 Nov 2017 18:48:27 +0100 Subject: [PATCH] Fixes crash that could happen when performing compile and run and start debug at approximately the same time. Project objects are now kept alive for as long as they are needed. Also removed Terminal::InProgress since it could lead to unsafe code in the future. --- CMakeLists.txt | 2 +- src/config.cc | 2 -- src/config.h | 1 - src/files.h | 3 +- src/project.cc | 67 ++++++++++++++++++++++-------------------- src/project.h | 10 +++---- src/source_clang.cc | 4 --- src/source_clang.h | 2 -- src/terminal.cc | 62 ++------------------------------------ src/terminal.h | 26 ---------------- tests/stubs/project.cc | 2 +- 11 files changed, 45 insertions(+), 136 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7c8246a..b76fb35 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required (VERSION 2.8.8) project(juci) -set(JUCI_VERSION "1.3.2") +set(JUCI_VERSION "1.3.2.1") set(CPACK_PACKAGE_NAME "jucipp") set(CPACK_PACKAGE_CONTACT "Ole Christian Eidheim ") diff --git a/src/config.cc b/src/config.cc index 59bf54c..92ae884 100644 --- a/src/config.cc +++ b/src/config.cc @@ -205,6 +205,4 @@ void Config::read(const boost::property_tree::ptree &cfg) { terminal.history_size=cfg.get("terminal.history_size"); terminal.font=cfg.get("terminal.font"); - - terminal.show_progress=cfg.get("terminal.show_progress"); } diff --git a/src/config.h b/src/config.h index 3164754..d357ec1 100644 --- a/src/config.h +++ b/src/config.h @@ -27,7 +27,6 @@ public: public: int history_size; std::string font; - bool show_progress; }; class Project { diff --git a/src/files.h b/src/files.h index 0c5bcbe..6a370f8 100644 --- a/src/files.h +++ b/src/files.h @@ -85,8 +85,7 @@ R"RAW( "terminal": { "history_size": 1000, "font_comment": "Use \"\" to use source.font with slightly smaller size", - "font": "", - "show_progress": false + "font": "" }, "keybindings": { "preferences": "comma", diff --git a/src/project.cc b/src/project.cc index e52c1ab..09c1f76 100644 --- a/src/project.cc +++ b/src/project.cc @@ -20,7 +20,7 @@ std::atomic Project::compiling(false); std::atomic Project::debugging(false); std::pair > Project::debug_stop; std::string Project::debug_status; -std::unique_ptr Project::current; +std::shared_ptr Project::current; #ifdef JUCI_ENABLE_DEBUG std::unordered_map Project::Clang::debug_options; #endif @@ -130,7 +130,7 @@ void Project::debug_update_stop() { } } -std::unique_ptr Project::create() { +std::shared_ptr Project::create() { std::unique_ptr build; if(auto view=Notebook::get().get_current_view()) { @@ -138,22 +138,22 @@ std::unique_ptr Project::create() { if(view->language) { auto language_id=view->language->get_id(); if(language_id=="markdown") - return std::unique_ptr(new Project::Markdown(std::move(build))); + return std::shared_ptr(new Project::Markdown(std::move(build))); if(language_id=="python") - return std::unique_ptr(new Project::Python(std::move(build))); + return std::shared_ptr(new Project::Python(std::move(build))); if(language_id=="js") - return std::unique_ptr(new Project::JavaScript(std::move(build))); + return std::shared_ptr(new Project::JavaScript(std::move(build))); if(language_id=="html") - return std::unique_ptr(new Project::HTML(std::move(build))); + return std::shared_ptr(new Project::HTML(std::move(build))); } } else build=Build::create(Directories::get().path); if(dynamic_cast(build.get()) || dynamic_cast(build.get())) - return std::unique_ptr(new Project::Clang(std::move(build))); + return std::shared_ptr(new Project::Clang(std::move(build))); else - return std::unique_ptr(new Project::Base(std::move(build))); + return std::shared_ptr(new Project::Base(std::move(build))); } std::pair Project::Base::get_run_arguments() { @@ -239,17 +239,18 @@ void Project::Clang::compile() { if(default_build_path.empty() || !build->update_default()) return; + compiling=true; + if(Config::get().project.clear_terminal_on_compile) Terminal::get().clear(); - compiling=true; Terminal::get().print("Compiling project "+filesystem::get_short_path(build->project_path).string()+"\n"); std::string compile_command; if(dynamic_cast(build.get())) compile_command=Config::get().project.cmake.compile_command; else if(dynamic_cast(build.get())) compile_command=Config::get().project.meson.compile_command; - Terminal::get().async_process(compile_command, default_build_path, [this](int exit_status) { + Terminal::get().async_process(compile_command, default_build_path, [](int exit_status) { compiling=false; }); } @@ -277,20 +278,21 @@ void Project::Clang::compile_and_run() { arguments=filesystem::escape_argument(filesystem::get_short_path(executable).string()); } + compiling=true; + if(Config::get().project.clear_terminal_on_compile) Terminal::get().clear(); - compiling=true; Terminal::get().print("Compiling and running "+arguments+"\n"); std::string compile_command; if(dynamic_cast(build.get())) compile_command=Config::get().project.cmake.compile_command; else if(dynamic_cast(build.get())) compile_command=Config::get().project.meson.compile_command; - Terminal::get().async_process(compile_command, default_build_path, [this, arguments, project_path](int exit_status){ + Terminal::get().async_process(compile_command, default_build_path, [arguments, project_path](int exit_status){ compiling=false; if(exit_status==EXIT_SUCCESS) { - Terminal::get().async_process(arguments, project_path, [this, arguments](int exit_status){ + Terminal::get().async_process(arguments, project_path, [arguments](int exit_status){ Terminal::get().async_print(arguments+" returned: "+std::to_string(exit_status)+'\n'); }); } @@ -416,21 +418,22 @@ void Project::Clang::debug_start() { *run_arguments=filesystem::escape_argument(filesystem::get_short_path(*run_arguments).string()); } + debugging=true; + if(Config::get().project.clear_terminal_on_compile) Terminal::get().clear(); - debugging=true; Terminal::get().print("Compiling and debugging "+*run_arguments+"\n"); std::string compile_command; if(dynamic_cast(build.get())) compile_command=Config::get().project.cmake.compile_command; else if(dynamic_cast(build.get())) compile_command=Config::get().project.meson.compile_command; - Terminal::get().async_process(compile_command, debug_build_path, [this, run_arguments, project_path](int exit_status){ + Terminal::get().async_process(compile_command, debug_build_path, [self=this->shared_from_this(), run_arguments, project_path](int exit_status){ if(exit_status!=EXIT_SUCCESS) debugging=false; else { - dispatcher.post([this, run_arguments, project_path] { + self->dispatcher.post([self, run_arguments, project_path] { std::vector > breakpoints; for(size_t c=0;cdispatcher.post([] { debug_update_status(""); }); }); @@ -463,7 +466,7 @@ void Project::Clang::debug_start() { static auto on_event_it=Debug::LLDB::get().on_event.end(); if(on_event_it!=Debug::LLDB::get().on_event.end()) Debug::LLDB::get().on_event.erase(on_event_it); - Debug::LLDB::get().on_event.emplace_back([this](const lldb::SBEvent &event) { + Debug::LLDB::get().on_event.emplace_back([self](const lldb::SBEvent &event) { std::string status; boost::filesystem::path stop_path; unsigned stop_line=0, stop_column=0; @@ -499,7 +502,7 @@ void Project::Clang::debug_start() { } } - dispatcher.post([this, status=std::move(status), stop_path=std::move(stop_path), stop_line, stop_column] { + self->dispatcher.post([status=std::move(status), stop_path=std::move(stop_path), stop_line, stop_column] { debug_update_status(status); Project::debug_stop.first=stop_path; Project::debug_stop.second.first=stop_line; @@ -585,7 +588,7 @@ void Project::Clang::debug_backtrace() { } } - SelectionDialog::get()->on_select=[this, rows=std::move(rows)](unsigned int index, const std::string &text, bool hide_window) { + SelectionDialog::get()->on_select=[rows=std::move(rows)](unsigned int index, const std::string &text, bool hide_window) { if(index>=rows.size()) return; auto frame=rows[index]; @@ -630,7 +633,7 @@ void Project::Clang::debug_show_variables() { SelectionDialog::get()->add_row(row); } - SelectionDialog::get()->on_select=[this, rows](unsigned int index, const std::string &text, bool hide_window) { + SelectionDialog::get()->on_select=[rows](unsigned int index, const std::string &text, bool hide_window) { if(index>=rows->size()) return; auto variable=(*rows)[index]; @@ -646,18 +649,18 @@ void Project::Clang::debug_show_variables() { Info::get().print("Debugger did not find declaration for the variable: "+variable.name); }; - SelectionDialog::get()->on_hide=[this]() { - debug_variable_tooltips.hide(); - debug_variable_tooltips.clear(); + SelectionDialog::get()->on_hide=[self=this->shared_from_this()]() { + self->debug_variable_tooltips.hide(); + self->debug_variable_tooltips.clear(); }; - SelectionDialog::get()->on_changed=[this, rows, view, iter](unsigned int index, const std::string &text) { + SelectionDialog::get()->on_changed=[self=this->shared_from_this(), rows, view, iter](unsigned int index, const std::string &text) { if(index>=rows->size()) { - debug_variable_tooltips.hide(); + self->debug_variable_tooltips.hide(); return; } - debug_variable_tooltips.clear(); - auto create_tooltip_buffer=[this, rows, view, index]() { + self->debug_variable_tooltips.clear(); + auto create_tooltip_buffer=[rows, view, index]() { auto variable=(*rows)[index]; auto tooltip_buffer=view?Gtk::TextBuffer::create(view->get_buffer()->get_tag_table()):Gtk::TextBuffer::create(); @@ -679,11 +682,11 @@ void Project::Clang::debug_show_variables() { }; if(view) - debug_variable_tooltips.emplace_back(create_tooltip_buffer, view, view->get_buffer()->create_mark(iter), view->get_buffer()->create_mark(iter)); + self->debug_variable_tooltips.emplace_back(create_tooltip_buffer, view, view->get_buffer()->create_mark(iter), view->get_buffer()->create_mark(iter)); else - debug_variable_tooltips.emplace_back(create_tooltip_buffer); + self->debug_variable_tooltips.emplace_back(create_tooltip_buffer); - debug_variable_tooltips.show(true); + self->debug_variable_tooltips.show(true); }; if(view) diff --git a/src/project.h b/src/project.h index 0501653..6b06c6d 100644 --- a/src/project.h +++ b/src/project.h @@ -26,7 +26,7 @@ namespace Project { void debug_activate_menu_items(); void debug_update_stop(); - class Base { + class Base : public std::enable_shared_from_this { protected: #ifdef JUCI_ENABLE_DEBUG class DebugOptions : public Gtk::Popover { @@ -42,6 +42,8 @@ namespace Project { std::unique_ptr build; + Dispatcher dispatcher; + virtual std::pair get_run_arguments(); virtual void compile(); virtual void compile_and_run(); @@ -77,8 +79,6 @@ namespace Project { }; static std::unordered_map debug_options; #endif - - Dispatcher dispatcher; public: Clang(std::unique_ptr &&build) : Base(std::move(build)) {} ~Clang() { dispatcher.disconnect(); } @@ -139,8 +139,8 @@ namespace Project { void compile_and_run() override; }; - std::unique_ptr create(); - extern std::unique_ptr current; + std::shared_ptr create(); + extern std::shared_ptr current; }; #endif // JUCI_PROJECT_H_ diff --git a/src/source_clang.cc b/src/source_clang.cc index 9c0c15f..cdd259c 100644 --- a/src/source_clang.cc +++ b/src/source_clang.cc @@ -27,7 +27,6 @@ Source::ClangViewParse::ClangViewParse(const boost::filesystem::path &file_path, } configure(); - parsing_in_progress=Terminal::get().print_in_progress("Parsing "+file_path.string()); parse_initialize(); get_buffer()->signal_changed().connect([this]() { @@ -142,7 +141,6 @@ void Source::ClangViewParse::parse_initialize() { if(this->language && (this->language->get_id()=="chdr" || this->language->get_id()=="cpphdr")) clangmm::remove_include_guard(parse_thread_buffer_raw); auto status=clang_tu->reparse(parse_thread_buffer_raw); - parsing_in_progress->done("done"); if(status==0) { auto expected=ParseProcessState::PROCESSING; if(parse_process_state.compare_exchange_strong(expected, ParseProcessState::POSTPROCESSING)) { @@ -183,7 +181,6 @@ void Source::ClangViewParse::parse_initialize() { status_diagnostics=std::make_tuple(0, 0, 0); if(update_status_diagnostics) update_status_diagnostics(this); - parsing_in_progress->cancel("failed"); }); } } @@ -1697,7 +1694,6 @@ void Source::ClangView::full_reparse() { void Source::ClangView::async_delete() { delayed_show_arguments_connection.disconnect(); delayed_tag_similar_identifiers_connection.disconnect(); - parsing_in_progress->cancel("canceled, freeing resources in the background"); views.erase(this); std::set project_paths_in_use; diff --git a/src/source_clang.h b/src/source_clang.h index 933eaad..e424180 100644 --- a/src/source_clang.h +++ b/src/source_clang.h @@ -32,8 +32,6 @@ namespace Source { std::vector> clang_tokens_offsets; sigc::connection delayed_reparse_connection; - std::shared_ptr parsing_in_progress; - void show_diagnostic_tooltips(const Gdk::Rectangle &rectangle) override; void show_type_tooltips(const Gdk::Rectangle &rectangle) override; diff --git a/src/terminal.cc b/src/terminal.cc index ed2deda..b64901c 100644 --- a/src/terminal.cc +++ b/src/terminal.cc @@ -5,44 +5,8 @@ #include "notebook.h" #include "filesystem.h" #include - -Terminal::InProgress::InProgress(const std::string& start_msg): stop(false) { - if(Config::get().terminal.show_progress) - start(start_msg); - else - stop=true; -} - -Terminal::InProgress::~InProgress() { - stop=true; - if(wait_thread.joinable()) - wait_thread.join(); -} - -void Terminal::InProgress::start(const std::string& msg) { - line_nr=Terminal::get().print(msg+"...\n"); - wait_thread=std::thread([this](){ - size_t c=0; - while(!stop) { - if(c%100==0) - Terminal::get().async_print(line_nr-1, "."); - std::this_thread::sleep_for(std::chrono::milliseconds(10)); - c++; - } - }); -} - -void Terminal::InProgress::done(const std::string& msg) { - bool expected=false; - if(stop.compare_exchange_strong(expected, true)) - Terminal::get().async_print(line_nr-1, msg); -} - -void Terminal::InProgress::cancel(const std::string& msg) { - bool expected=false; - if(stop.compare_exchange_strong(expected, true)) - Terminal::get().async_print(line_nr-1, msg); -} +#include +#include Terminal::Terminal() { bold_tag=get_buffer()->create_tag(); @@ -319,21 +283,6 @@ size_t Terminal::print(const std::string &message, bool bold){ return static_cast(get_buffer()->end().get_line())+deleted_lines; } -std::shared_ptr Terminal::print_in_progress(std::string start_msg) { - auto in_progress=std::shared_ptr(new Terminal::InProgress(start_msg), [this](Terminal::InProgress *in_progress) { - { - std::unique_lock lock(in_progresses_mutex); - in_progresses.erase(in_progress); - } - delete in_progress; - }); - { - std::unique_lock lock(in_progresses_mutex); - in_progresses.emplace(in_progress.get()); - } - return in_progress; -} - void Terminal::async_print(const std::string &message, bool bold) { dispatcher.post([this, message, bold] { Terminal::get().print(message, bold); @@ -379,13 +328,6 @@ void Terminal::configure() { } void Terminal::clear() { - { - std::unique_lock lock(in_progresses_mutex); - for(auto &in_progress: in_progresses) - in_progress->stop=true; - } - while(Gtk::Main::events_pending()) - Gtk::Main::iteration(false); get_buffer()->set_text(""); } diff --git a/src/terminal.h b/src/terminal.h index 2020cd6..2b1628e 100644 --- a/src/terminal.h +++ b/src/terminal.h @@ -5,34 +5,12 @@ #include #include "gtkmm.h" #include -#include -#include #include #include "process.hpp" #include "dispatcher.h" -#include -#include #include class Terminal : public Gtk::TextView { -public: - class InProgress { - friend class Terminal; - public: - InProgress(const std::string& start_msg); - ~InProgress(); - void done(const std::string& msg); - void cancel(const std::string& msg); - private: - void start(const std::string& msg); - size_t line_nr; - - std::atomic stop; - - std::thread wait_thread; - }; - -private: Terminal(); public: static Terminal &get() { @@ -47,7 +25,6 @@ public: void kill_async_processes(bool force=false); size_t print(const std::string &message, bool bold=false); - std::shared_ptr print_in_progress(std::string start_msg); void async_print(const std::string &message, bool bold=false); void async_print(size_t line_nr, const std::string &message); @@ -72,9 +49,6 @@ private: std::vector> processes; std::mutex processes_mutex; Glib::ustring stdin_buffer; - - std::unordered_set in_progresses; - std::mutex in_progresses_mutex; }; #endif // JUCI_TERMINAL_H_ diff --git a/tests/stubs/project.cc b/tests/stubs/project.cc index 3487dbf..da4d3ca 100644 --- a/tests/stubs/project.cc +++ b/tests/stubs/project.cc @@ -1,3 +1,3 @@ #include "project.h" -std::unique_ptr Project::current; +std::shared_ptr Project::current;