From 3f70e096f27a086c333cc7c396f1797ee2d267b2 Mon Sep 17 00:00:00 2001 From: James Date: Wed, 16 Oct 2024 12:06:58 +0700 Subject: [PATCH] fix: crash when calling process kill api Also remove memory alloc for DownloadService, instead, injected. --- engine/commands/chat_cmd.cc | 14 ++++---------- engine/commands/chat_cmd.h | 7 +++++-- engine/commands/engine_get_cmd.h | 4 ++-- engine/commands/engine_install_cmd.h | 4 ++-- engine/commands/engine_list_cmd.h | 4 ++-- engine/commands/engine_uninstall_cmd.h | 4 ++-- engine/commands/model_del_cmd.h | 4 ++-- engine/commands/model_pull_cmd.h | 4 ++-- engine/commands/run_cmd.h | 7 ++++--- engine/controllers/command_line_parser.cc | 18 +++++++++--------- engine/controllers/command_line_parser.h | 1 + engine/controllers/process_manager.cc | 8 -------- engine/controllers/process_manager.h | 7 ------- engine/main.cc | 6 +++--- engine/services/download_service.cc | 19 +++++++------------ engine/services/download_service.h | 5 +++-- 16 files changed, 48 insertions(+), 68 deletions(-) diff --git a/engine/commands/chat_cmd.cc b/engine/commands/chat_cmd.cc index 3f5f221bc..381aa91dc 100644 --- a/engine/commands/chat_cmd.cc +++ b/engine/commands/chat_cmd.cc @@ -1,17 +1,11 @@ #include "chat_cmd.h" -#include "httplib.h" - -#include "database/models.h" -#include "model_status_cmd.h" -#include "server_start_cmd.h" -#include "trantor/utils/Logger.h" -#include "utils/logging_utils.h" #include "run_cmd.h" namespace commands { void ChatCmd::Exec(const std::string& host, int port, - const std::string& model_handle) { - RunCmd rc(host, port, model_handle); + const std::string& model_handle, + std::shared_ptr download_service) { + RunCmd rc(host, port, model_handle, download_service); rc.Exec(true /*chat_flag*/); } -}; // namespace commands \ No newline at end of file +}; // namespace commands diff --git a/engine/commands/chat_cmd.h b/engine/commands/chat_cmd.h index 5abcb3cd6..597a0d752 100644 --- a/engine/commands/chat_cmd.h +++ b/engine/commands/chat_cmd.h @@ -1,9 +1,12 @@ #pragma once + #include +#include "services/download_service.h" namespace commands { class ChatCmd { public: - void Exec(const std::string& host, int port, const std::string& model_handle); + void Exec(const std::string& host, int port, const std::string& model_handle, + std::shared_ptr download_service); }; -} // namespace commands \ No newline at end of file +} // namespace commands diff --git a/engine/commands/engine_get_cmd.h b/engine/commands/engine_get_cmd.h index 0a58b7e37..723c13ef9 100644 --- a/engine/commands/engine_get_cmd.h +++ b/engine/commands/engine_get_cmd.h @@ -5,8 +5,8 @@ namespace commands { class EngineGetCmd { public: - explicit EngineGetCmd() - : engine_service_{EngineService(std::make_shared())} {}; + explicit EngineGetCmd(std::shared_ptr download_service) + : engine_service_{EngineService(download_service)} {}; void Exec(const std::string& engineName) const; diff --git a/engine/commands/engine_install_cmd.h b/engine/commands/engine_install_cmd.h index 8fc0ce6f0..199d4d319 100644 --- a/engine/commands/engine_install_cmd.h +++ b/engine/commands/engine_install_cmd.h @@ -7,8 +7,8 @@ namespace commands { class EngineInstallCmd { public: - explicit EngineInstallCmd() - : engine_service_{EngineService(std::make_shared())} {}; + explicit EngineInstallCmd(std::shared_ptr download_service) + : engine_service_{EngineService(download_service)} {}; void Exec(const std::string& engine, const std::string& version = "latest", const std::string& src = ""); diff --git a/engine/commands/engine_list_cmd.h b/engine/commands/engine_list_cmd.h index be38bdd2d..390c8af4f 100644 --- a/engine/commands/engine_list_cmd.h +++ b/engine/commands/engine_list_cmd.h @@ -5,8 +5,8 @@ namespace commands { class EngineListCmd { public: - explicit EngineListCmd() - : engine_service_{EngineService(std::make_shared())} {}; + explicit EngineListCmd(std::shared_ptr download_service) + : engine_service_{EngineService(download_service)} {}; bool Exec(); diff --git a/engine/commands/engine_uninstall_cmd.h b/engine/commands/engine_uninstall_cmd.h index 46b1bd923..d5455566d 100644 --- a/engine/commands/engine_uninstall_cmd.h +++ b/engine/commands/engine_uninstall_cmd.h @@ -7,8 +7,8 @@ namespace commands { class EngineUninstallCmd { public: - explicit EngineUninstallCmd() - : engine_service_{EngineService(std::make_shared())} {}; + explicit EngineUninstallCmd(std::shared_ptr download_service) + : engine_service_{EngineService(download_service)} {}; void Exec(const std::string& engine); diff --git a/engine/commands/model_del_cmd.h b/engine/commands/model_del_cmd.h index 38f172ad2..96154d3dd 100644 --- a/engine/commands/model_del_cmd.h +++ b/engine/commands/model_del_cmd.h @@ -7,8 +7,8 @@ namespace commands { class ModelDelCmd { public: - explicit ModelDelCmd() - : model_service_{ModelService(std::make_shared())} {}; + explicit ModelDelCmd(std::shared_ptr download_service) + : model_service_{ModelService(download_service)} {}; void Exec(const std::string& model_handle); diff --git a/engine/commands/model_pull_cmd.h b/engine/commands/model_pull_cmd.h index 56d8a282a..3586b3cd4 100644 --- a/engine/commands/model_pull_cmd.h +++ b/engine/commands/model_pull_cmd.h @@ -6,8 +6,8 @@ namespace commands { class ModelPullCmd { public: - explicit ModelPullCmd() - : model_service_{ModelService(std::make_shared())} {}; + explicit ModelPullCmd(std::shared_ptr download_service) + : model_service_{ModelService(download_service)} {}; void Exec(const std::string& input); private: diff --git a/engine/commands/run_cmd.h b/engine/commands/run_cmd.h index 4bbcbf549..b035a54d9 100644 --- a/engine/commands/run_cmd.h +++ b/engine/commands/run_cmd.h @@ -7,12 +7,13 @@ namespace commands { class RunCmd { public: - explicit RunCmd(std::string host, int port, std::string model_handle) + explicit RunCmd(std::string host, int port, std::string model_handle, + std::shared_ptr download_service) : host_{std::move(host)}, port_{port}, model_handle_{std::move(model_handle)}, - engine_service_{EngineService(std::make_shared())}, - model_service_{ModelService(std::make_shared())} {}; + engine_service_{EngineService(download_service)}, + model_service_{ModelService(download_service)} {}; void Exec(bool chat_flag); diff --git a/engine/controllers/command_line_parser.cc b/engine/controllers/command_line_parser.cc index df83db688..559be7ba7 100644 --- a/engine/controllers/command_line_parser.cc +++ b/engine/controllers/command_line_parser.cc @@ -128,7 +128,7 @@ void CommandLineParser::SetupCommonCommands() { return; } try { - commands::ModelPullCmd().Exec(cml_data_.model_id); + commands::ModelPullCmd(download_service_).Exec(cml_data_.model_id); } catch (const std::exception& e) { CLI_LOG(e.what()); } @@ -150,7 +150,7 @@ void CommandLineParser::SetupCommonCommands() { } commands::RunCmd rc(cml_data_.config.apiServerHost, std::stoi(cml_data_.config.apiServerPort), - cml_data_.model_id); + cml_data_.model_id, download_service_); rc.Exec(cml_data_.chat_flag); }); @@ -175,7 +175,7 @@ void CommandLineParser::SetupCommonCommands() { if (cml_data_.msg.empty()) { commands::ChatCmd().Exec(cml_data_.config.apiServerHost, std::stoi(cml_data_.config.apiServerPort), - cml_data_.model_id); + cml_data_.model_id, download_service_); } else { commands::ChatCompletionCmd(model_service_) .Exec(cml_data_.config.apiServerHost, @@ -285,7 +285,7 @@ void CommandLineParser::SetupModelCommands() { CLI_LOG(model_del_cmd->help()); return; }; - commands::ModelDelCmd().Exec(cml_data_.model_id); + commands::ModelDelCmd(download_service_).Exec(cml_data_.model_id); }); std::string model_alias; @@ -358,8 +358,7 @@ void CommandLineParser::SetupEngineCommands() { list_engines_cmd->callback([this]() { if (std::exchange(executed_, true)) return; - commands::EngineListCmd command; - command.Exec(); + commands::EngineListCmd(download_service_).Exec(); }); auto install_cmd = engines_cmd->add_subcommand("install", "Install engine"); @@ -478,7 +477,8 @@ void CommandLineParser::EngineInstall(CLI::App* parent, if (std::exchange(executed_, true)) return; try { - commands::EngineInstallCmd().Exec(engine_name, version, src); + commands::EngineInstallCmd(download_service_) + .Exec(engine_name, version, src); } catch (const std::exception& e) { CTL_ERR(e.what()); } @@ -496,7 +496,7 @@ void CommandLineParser::EngineUninstall(CLI::App* parent, if (std::exchange(executed_, true)) return; try { - commands::EngineUninstallCmd().Exec(engine_name); + commands::EngineUninstallCmd(download_service_).Exec(engine_name); } catch (const std::exception& e) { CTL_ERR(e.what()); } @@ -528,7 +528,7 @@ void CommandLineParser::EngineGet(CLI::App* parent) { engine_get_cmd->callback([this, engine_name] { if (std::exchange(executed_, true)) return; - commands::EngineGetCmd().Exec(engine_name); + commands::EngineGetCmd(download_service_).Exec(engine_name); }); } } diff --git a/engine/controllers/command_line_parser.h b/engine/controllers/command_line_parser.h index 96cf886bf..0fc3f1aa7 100644 --- a/engine/controllers/command_line_parser.h +++ b/engine/controllers/command_line_parser.h @@ -5,6 +5,7 @@ #include "services/engine_service.h" #include "services/model_service.h" #include "utils/config_yaml_utils.h" + class CommandLineParser { public: CommandLineParser(); diff --git a/engine/controllers/process_manager.cc b/engine/controllers/process_manager.cc index efacc3c50..8373f08fe 100644 --- a/engine/controllers/process_manager.cc +++ b/engine/controllers/process_manager.cc @@ -1,6 +1,5 @@ #include "process_manager.h" #include "utils/cortex_utils.h" -#include "utils/logging_utils.h" #include #include @@ -9,13 +8,6 @@ void ProcessManager::destroy( const HttpRequestPtr& req, std::function&& callback) { - auto destroy_result = download_service_->Destroy(); - if (destroy_result.has_error()) { - CTL_ERR("Failed to destroy download service: " + destroy_result.error()); - } else { - CTL_INF("Download service stopped!"); - } - app().quit(); Json::Value ret; ret["message"] = "Program is exitting, goodbye!"; diff --git a/engine/controllers/process_manager.h b/engine/controllers/process_manager.h index 4cf4f81d4..f62c51abb 100644 --- a/engine/controllers/process_manager.h +++ b/engine/controllers/process_manager.h @@ -2,7 +2,6 @@ #include #include -#include "services/download_service.h" using namespace drogon; @@ -12,12 +11,6 @@ class ProcessManager : public drogon::HttpController { METHOD_ADD(ProcessManager::destroy, "/destroy", Delete); METHOD_LIST_END - explicit ProcessManager(std::shared_ptr download_service) - : download_service_{download_service} {} - void destroy(const HttpRequestPtr& req, std::function&& callback); - - private: - std::shared_ptr download_service_; }; diff --git a/engine/main.cc b/engine/main.cc index 2667c1202..339b7c12a 100644 --- a/engine/main.cc +++ b/engine/main.cc @@ -51,7 +51,7 @@ void RunServer() { std::filesystem::create_directories( std::filesystem::path(config.logFolderPath) / std::filesystem::path(cortex_utils::logs_folder)); - trantor::FileLogger asyncFileLogger; + static trantor::FileLogger asyncFileLogger; asyncFileLogger.setFileName( (std::filesystem::path(config.logFolderPath) / std::filesystem::path(cortex_utils::logs_base_name)) @@ -93,7 +93,7 @@ void RunServer() { auto engine_ctl = std::make_shared(engine_service); auto model_ctl = std::make_shared(model_service); auto event_ctl = std::make_shared(event_queue_ptr); - auto pm_ctl = std::make_shared(download_service); + auto pm_ctl = std::make_shared(); drogon::app().registerController(engine_ctl); drogon::app().registerController(model_ctl); @@ -126,7 +126,7 @@ int main(int argc, char* argv[]) { if (strcmp(argv[i], "--config_file_path") == 0) { file_manager_utils::cortex_config_file_path = argv[i + 1]; - } else if(strcmp(argv[i], "--data_folder_path") == 0) { + } else if (strcmp(argv[i], "--data_folder_path") == 0) { file_manager_utils::cortex_data_folder_path = argv[i + 1]; } } diff --git a/engine/services/download_service.cc b/engine/services/download_service.cc index a1905eefb..3966d8a98 100644 --- a/engine/services/download_service.cc +++ b/engine/services/download_service.cc @@ -294,8 +294,12 @@ void DownloadService::ProcessTask(DownloadTask& task) { } } while (still_running); - ProcessCompletedTransfers(); + if (stop_flag_) { + CTL_INF("Download service is stopping.."); + return; + } + ProcessCompletedTransfers(); for (auto handle : task_handles) { curl_multi_remove_handle(multi_handle_, handle); curl_easy_cleanup(handle); @@ -304,14 +308,12 @@ void DownloadService::ProcessTask(DownloadTask& task) { // if terminate by API calling and not from process stopping, we emit // DownloadStopped event - if (is_terminated && !stop_flag_) { + if (is_terminated) { event_queue_->enqueue( EventType::DownloadEvent, DownloadEvent{.type_ = DownloadEventType::DownloadStopped, .download_task_ = task}); - } - - if (!is_terminated) { + } else { RemoveTaskFromStopList(task.id); CTL_INF("Executing callback.."); ExecuteCallback(task); @@ -402,10 +404,3 @@ void DownloadService::ExecuteCallback(const DownloadTask& task) { callbacks_.erase(it); } } - -cpp::result DownloadService::Destroy() { - CTL_INF("Destroying download service.."); - stop_flag_ = true; - queue_condition_.notify_one(); - return {}; -} diff --git a/engine/services/download_service.h b/engine/services/download_service.h index 605c517d0..af46e2d47 100644 --- a/engine/services/download_service.h +++ b/engine/services/download_service.h @@ -50,6 +50,9 @@ class DownloadService { } } + DownloadService(const DownloadService&) = delete; + DownloadService& operator=(const DownloadService&) = delete; + /** * Adding new download task to the queue. Asynchronously. This function should * be used by HTTP API. @@ -74,8 +77,6 @@ class DownloadService { cpp::result StopTask(const std::string& task_id); - cpp::result Destroy(); - private: struct DownloadingData { std::string item_id;