Merge pull request #3141 from ERGO-Code/od/resetGlobalScheduler

Remove resetGlobalScheduler calls from C API

Author: jajhall

.github/julia/build_tarballs.jl

@@ -12,9 +12,6 @@ version = VersionNumber(ENV["HIGHS_RELEASE"])
 sources = [GitSource(ENV["HIGHS_URL"], ENV["HIGHS_COMMIT"])]
 
 script = raw"""
-export BUILD_SHARED="ON"
-export BUILD_STATIC="OFF"
-
 cd $WORKSPACE/srcdir/HiGHS
 
 # Remove system CMake to use the jll version
@@ -23,19 +20,21 @@ apk del cmake
 rm -rf build
 mkdir build
 
-# Do fully static build only on Windows
-if [[ "${BUILD_SHARED}" == "OFF" ]] && [[ "${target}" == *-mingw* ]]; then
-    export CXXFLAGS="-static"
+if [[ "${target}" == *-mingw* ]]; then
+    LBT=blastrampoline-5
+else
+    LBT=blastrampoline
 fi
 
 cmake -S . -B build \
     -DCMAKE_INSTALL_PREFIX=${prefix} \
     -DCMAKE_TOOLCHAIN_FILE=${CMAKE_TARGET_TOOLCHAIN} \
     -DCMAKE_BUILD_TYPE=Release \
-    -DBUILD_SHARED_LIBS=${BUILD_SHARED} \
-    -DZLIB_USE_STATIC_LIBS=${BUILD_STATIC} \
-    -DHIPO=ON -DBUILD_SHARED_EXTRAS_LIB=OFF \
-    -DBLAS_LIBRARIES="${libdir}/libopenblas.${dlext}"
+    -DBUILD_SHARED_LIBS=ON \
+    -DHIPO=ON \
+    -DBUILD_SHARED_EXTRAS_LIB=OFF \
+    -DBLA_VENDOR=blastrampoline \
+    -DBLAS_LIBRARIES=\"${LBT}\"
 
 if [[ "${target}" == *-linux-* ]]; then
     make -C build -j ${nproc}
@@ -62,7 +61,7 @@ platforms = expand_cxxstring_abis(platforms)
 dependencies = [
     Dependency("CompilerSupportLibraries_jll"),
     Dependency("Zlib_jll"),
-    Dependency("OpenBLAS32_jll"),
+    Dependency("libblastrampoline_jll"),
     HostBuildDependency(PackageSpec(; name="CMake_jll")),
 ]
 
@@ -76,5 +75,5 @@ build_tarballs(
     products,
     dependencies;
     preferred_gcc_version = v"11",
-    julia_compat = "1.6",
+    julia_compat = "1.10",
 )

.github/workflows/julia-tests.yml

@@ -1,12 +1,9 @@
 name: JuliaCompileAndTest
-
 on: [push, pull_request]
-# needed to allow julia-actions/cache to delete old caches that it has created
-
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
-
+# needed to allow julia-actions/cache to delete old caches that it has created
 permissions:
   actions: write
   contents: read
@@ -18,7 +15,7 @@ jobs:
       matrix:
         triplet: ['x86_64-linux-gnu-cxx11', 'aarch64-apple-darwin', 'x86_64-w64-mingw32-cxx11']
     steps:
-      - uses: actions/checkout@v6
+      - uses: actions/checkout@v7
       - uses: julia-actions/setup-julia@v3
         with:
           version: "1.7"
@@ -64,7 +61,7 @@ jobs:
             os: 'windows-latest'
             arch: x64
     steps:
-      - uses: actions/checkout@v6
+      - uses: actions/checkout@v7
       - uses: julia-actions/setup-julia@v3
         with:
           version: "1.10"
@@ -106,18 +103,7 @@ jobs:
             "$(m[1]) = \"$(dir)\"\n",
           )
           Pkg.develop(; path = joinpath(ENV["GITHUB_WORKSPACE"], "HiGHS_jll"))
-      - shell: julia --color=yes {0}
-        run: |
-          import HiGHS_jll
-          file = joinpath(ENV["GITHUB_WORKSPACE"], "check", "instances", "flugpl.mps")
-          run(`$(HiGHS_jll.highs()) --solver=hipo $file`)
-          run(`$(HiGHS_jll.highs()) $file`)
       - shell: julia --color=yes {0}
         run: |
           using Pkg
-          # HiGHS uses (N+1)/2 threads. HiGHS.jl parallelises the tests and, in
-          # the default setting, it may use up to 4 parallel jobs. This means
-          # HiGHS may attempt to start up to 2(N+1) threads and we get unhandled
-          # exceptions on macOS and a SIGTERM on linux. To avoid these errors,
-          # opt out of running the tests in parallel.
-          Pkg.test("HiGHS"; test_args=["--parallel=false"])
+          Pkg.test("HiGHS")

check/TestHighsParallel.cpp

@@ -7,6 +7,7 @@
 #include <iostream>
 
 #include "catch.hpp"
+#include "interfaces/highs_c_api.h"
 #include "matrix_multiplication.hpp"
 #include "parallel/HighsParallel.h"
 
@@ -305,4 +306,52 @@ TEST_CASE("CancelNestedTasks", "[parallel]") {
   }
 
   HighsTaskExecutor::shutdown();
+}
+
+TEST_CASE("ParallelCApi", "[parallel]") {
+  // This test exposes a failure in the C api when creating and destroying Highs
+  // instances on separate threads.
+  // One thread solves a mip, while the other loses some time before destroying
+  // the instance. If this causes the scheduler to be terminated (i.e., if
+  // Highs_destroy calls resetGlobalScheduler) while the first thread is still
+  // using it, the first thread may crash, hang, or terminate with wrong
+  // results.
+
+  highs::parallel::initialize_scheduler();
+
+  std::string model = std::string(HIGHS_DIR) + "/check/instances/egout.mps";
+  const double expected_obj = 568.1007;
+  double actual_obj;
+  double total = 0;
+
+  {
+    highs::parallel::TaskGroup tg;
+
+    tg.spawn([&]() {
+      void* highs = Highs_create();
+      Highs_setBoolOptionValue(highs, "output_flag", dev_run);
+      Highs_setStringOptionValue(highs, "parallel", "on");
+      Highs_readModel(highs, model.c_str());
+      Highs_run(highs);
+      Highs_getDoubleInfoValue(highs, "objective_function_value", &actual_obj);
+      Highs_destroy(highs);
+    });
+
+    tg.spawn([&]() {
+      void* highs = Highs_create();
+      for (int i = 0; i < 1e6; ++i) {
+        total += (double)i * i * i;
+      }
+      Highs_destroy(highs);
+    });
+
+    tg.taskWait();
+
+    REQUIRE(total > 1e18);
+    REQUIRE(std::abs(actual_obj - expected_obj) / std::abs(expected_obj) <
+            1e-4);
+  }
+  // TaskGroup destructor must be called before scheduler is killed
+
+  HighsTaskExecutor::shutdown(true);
 }

examples/call_highs_from_c_minimal.c

@@ -655,5 +655,6 @@ int main() {
   minimal_api();
   minimal_api_qp();
   full_api();
+  Highs_resetGlobalScheduler(1);
   return 0;
 }

highs/interfaces/highs_c_api.cpp

@@ -61,9 +61,6 @@ HighsInt Highs_lpCall(const HighsInt num_col, const HighsInt num_row,
       if (copy_row_basis) row_basis_status[i] = (HighsInt)basis.row_status[i];
     }
   }
-
-  highs.resetGlobalScheduler(true);
-
   return (HighsInt)status;
 }
 
@@ -106,9 +103,6 @@ HighsInt Highs_mipCall(const HighsInt num_col, const HighsInt num_row,
         row_value[i] = solution.row_value[i];
     }
   }
-
-  highs.resetGlobalScheduler(true);
-
   return (HighsInt)status;
 }
 
@@ -165,16 +159,12 @@ HighsInt Highs_qpCall(
       if (copy_row_basis) row_basis_status[i] = (HighsInt)basis.row_status[i];
     }
   }
-
-  highs.resetGlobalScheduler(true);
-
   return (HighsInt)status;
 }
 
 void* Highs_create(void) { return new Highs(); }
 
 void Highs_destroy(void* highs) {
-  Highs::resetGlobalScheduler(true);
   delete (Highs*)highs;
 }