diff --git a/app/RunHighs.cpp b/app/RunHighs.cpp index 0b24fc44c0..840663e8d2 100644 --- a/app/RunHighs.cpp +++ b/app/RunHighs.cpp @@ -8,9 +8,23 @@ /**@file ../app/RunHighs.cpp * @brief HiGHS main */ +#include // For fclose + #include "Highs.h" #include "HighsRuntimeOptions.h" +int runHighsReturn(Highs& highs, const int status) { + // Close any log file explicitly + highs.closeLogFile(); + // Check that the log file has been closed + assert(highs.getOptions().log_options.log_stream == nullptr); + return status; +} + +int runHighsReturn(Highs& highs, const HighsStatus status) { + return runHighsReturn(highs, int(status)); +} + int main(int argc, char** argv) { // Create the Highs instance. Highs highs; @@ -40,31 +54,31 @@ int main(int argc, char** argv) { app.parse(argc, argv); } catch (const CLI::CallForHelp& e) { std::cout << app.help() << std::endl; - return 0; + return runHighsReturn(highs, 0); } catch (const CLI::CallForAllHelp& e) { std::cout << app.help(); - return 0; + return runHighsReturn(highs, 0); } catch (const CLI::RequiredError& e) { std::cout << "Please specify filename in .mps|.lp|.ems format." << std::endl; - return (int)HighsStatus::kError; + return runHighsReturn(highs, HighsStatus::kError); } catch (const CLI::ExtrasError& e) { std::cout << e.what() << std::endl; std::cout << "Multiple files not supported." << std::endl; - return (int)HighsStatus::kError; + return runHighsReturn(highs, HighsStatus::kError); } catch (const CLI::ArgumentMismatch& e) { std::cout << e.what() << std::endl; std::cout << "Too many arguments provided. Please provide only one." << std::endl; - return (int)HighsStatus::kError; + return runHighsReturn(highs, HighsStatus::kError); } catch (const CLI::ParseError& e) { std::cout << e.what() << std::endl; // app.exit() should be called from main. - return app.exit(e); + return runHighsReturn(highs, app.exit(e)); } if (!loadOptions(app, log_options, cmd_options, loaded_options)) - return (int)HighsStatus::kError; + return runHighsReturn(highs, HighsStatus::kError); // Open the app log file - unless output_flag is false, to avoid // creating an empty file. It does nothing if its name is "". @@ -86,13 +100,13 @@ int main(int argc, char** argv) { HighsStatus read_status = highs.readModel(cmd_options.model_file); if (read_status == HighsStatus::kError) { highsLogUser(log_options, HighsLogType::kInfo, "Error loading file\n"); - return (int)read_status; + return runHighsReturn(highs, read_status); } if (options.write_presolved_model_file != "") { // Run presolve and write the presolved model to a file HighsStatus status = highs.presolve(); - if (status == HighsStatus::kError) return int(status); + if (status == HighsStatus::kError) return runHighsReturn(highs, status); HighsPresolveStatus model_presolve_status = highs.getModelPresolveStatus(); const bool ok_to_write = model_presolve_status == HighsPresolveStatus::kNotReduced || @@ -102,18 +116,18 @@ int main(int argc, char** argv) { if (!ok_to_write) { highsLogUser(log_options, HighsLogType::kInfo, "No presolved model to write to file\n"); - return int(status); + return runHighsReturn(highs, status); } status = highs.writePresolvedModel(options.write_presolved_model_file); - return int(status); + return runHighsReturn(highs, status); } // Solve the model HighsStatus run_status = highs.run(); - if (run_status == HighsStatus::kError) return int(run_status); + if (run_status == HighsStatus::kError) runHighsReturn(highs, run_status); // Shut down task executor for explicit release of memory. // Valgrind still reachable otherwise. highs.resetGlobalScheduler(true); - return (int)run_status; + return runHighsReturn(highs, run_status); } diff --git a/highs/Highs.h b/highs/Highs.h index 2beaa37948..7d2c21a538 100644 --- a/highs/Highs.h +++ b/highs/Highs.h @@ -42,13 +42,7 @@ const char* highsGithash(); class Highs { public: Highs(); - virtual ~Highs() { - FILE* log_stream = options_.log_options.log_stream; - if (log_stream != nullptr) { - assert(log_stream != stdout); - fclose(log_stream); - } - } + virtual ~Highs() { this->closeLogFile(); } /** * @brief Return the version as a string @@ -1216,6 +1210,11 @@ class Highs { */ HighsStatus openLogFile(const std::string& log_file = ""); + /** + * @brief Close any open log file + */ + HighsStatus closeLogFile(); + /** * @brief Interpret common qualifiers to string values */ diff --git a/highs/lp_data/Highs.cpp b/highs/lp_data/Highs.cpp index fdaa1080a2..0f730db2ac 100644 --- a/highs/lp_data/Highs.cpp +++ b/highs/lp_data/Highs.cpp @@ -4762,6 +4762,21 @@ HighsStatus Highs::openLogFile(const std::string& log_file) { return HighsStatus::kOk; } +HighsStatus Highs::closeLogFile() { + // Work with a copy of the pointer for brevity + FILE* log_stream = this->options_.log_options.log_stream; + if (log_stream != nullptr) { + assert(log_stream != stdout); + fflush(log_stream); + fclose(log_stream); + // Set the true log_stream to nullptr to give a test whether it + // has been closed (and avoid trying to close it again which + // causes an error) + this->options_.log_options.log_stream = nullptr; + } + return HighsStatus::kOk; +} + void Highs::resetGlobalScheduler(bool blocking) { HighsTaskExecutor::shutdown(blocking); } diff --git a/highs/lp_data/HighsOptions.cpp b/highs/lp_data/HighsOptions.cpp index 13ec2abdcb..393720d671 100644 --- a/highs/lp_data/HighsOptions.cpp +++ b/highs/lp_data/HighsOptions.cpp @@ -25,19 +25,22 @@ void highsOpenLogFile(HighsLogOptions& log_options, OptionStatus status = getOptionIndex(log_options, "log_file", option_records, index); assert(status == OptionStatus::kOk); - if (log_options.log_stream != NULL) { + if (log_options.log_stream != nullptr) { // Current log file stream is not null, so flush and close it fflush(log_options.log_stream); fclose(log_options.log_stream); + // Set the stream to null to give a test whether it has been + // closed (and avoid trying to close it again which causes an + // error) + log_options.log_stream = nullptr; } - if (log_file.compare("")) { - // New log file name is not empty, so open it, appending if - // possible + assert(!log_options.log_stream); + // If new log file name is not empty, open it, appending if possible + // + // If fopen fails then it returns nullptr, so log_file is open for + // writing or nullptr + if (log_file.compare("")) log_options.log_stream = fopen(log_file.c_str(), "a"); - } else { - // New log file name is empty, so set the stream to null - log_options.log_stream = NULL; - } OptionRecordString& option = *(OptionRecordString*)option_records[index]; option.assignvalue(log_file); }