Added Highs::closeLogFile() and calling this before returning from the command line executable to ensure that the log file is closed

Author: jajhall

app/RunHighs.cpp

@@ -8,9 +8,23 @@
 /**@file ../app/RunHighs.cpp
  * @brief HiGHS main
  */
+#include <cstdio>  // 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);
 }

highs/Highs.h

@@ -43,10 +43,14 @@ class Highs {
  public:
   Highs();
   virtual ~Highs() {
-    FILE* log_stream = options_.log_options.log_stream;
+    FILE* log_stream = this->options_.log_options.log_stream;
     if (log_stream != nullptr) {
       assert(log_stream != stdout);
-      fclose(log_stream);
+      // Was this, but only passing a copy of the pointer, so not
+      // closing the true log stream?
+      //
+      // fclose(log_stream);
+      fclose(this->options_.log_options.log_stream);
     }
   }
 
@@ -1216,6 +1220,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
    */

highs/lp_data/Highs.cpp

@@ -4762,6 +4762,25 @@ HighsStatus Highs::openLogFile(const std::string& log_file) {
   return HighsStatus::kOk;
 }
 
+HighsStatus Highs::closeLogFile() {
+  FILE* log_stream = this->options_.log_options.log_stream;
+  if (log_stream != nullptr) {
+    assert(log_stream != stdout);
+    // Using this, as in the original destructor of Highs, only
+    // passing a copy of the pointer, so not closing the true log
+    // stream?
+    //
+    // fclose(log_stream);
+    fclose(this->options_.log_options.log_stream);
+    // Set log_stream to nullptr to give a test whether the it has
+    // been closed (and avoid trying to close it again which causes an
+    // error).
+    this->options_.log_options.log_stream = nullptr;
+    assert(this->options_.log_options.log_stream == nullptr);
+  }
+  return HighsStatus::kOk;
+}
+
 void Highs::resetGlobalScheduler(bool blocking) {
   HighsTaskExecutor::shutdown(blocking);
 }