ERGO-Code / HiGHS

Linear optimization software
MIT License
992 stars 184 forks source link

An attempt to fix potential deadlock issues in Windows. #1923

Closed mathgeekcoder closed 2 months ago

mathgeekcoder commented 2 months ago

Removes synchronization with worker threads on shutdown. Also removes the "search" for the main executor in the worker threads.

Instead we simply pass the main executor to the thread as a parameter. We also pass the underlying shared_ptr to avoid potential edge cases where reference count drops to zero before some threads initialize.

I made the run_worker static to avoid any confusion about this vs executor->ptr, and so it uses the shared_ptr to reference the shared memory.

The last worker thread will delete the shared memory, via the shared_ptr reference count.

This might fix issues mentioned in #1886 and #1044.

mathgeekcoder commented 2 months ago

Sample C++ code that reproduces the error on windows.

std::array<double, 2> lb = { -kHighsInf, -kHighsInf };
std::array<double, 2> ub = {  kHighsInf,  kHighsInf };

std::array<HighsInt, 2> index = { 0, 1 };
std::array<double, 2> value = { 0, 1 };

std::array<double, 2> lower = { 2, 0 };
std::array<HighsInt, 2> starts = { 0, 2 };
std::array<HighsInt, 4> indices = { 0, 1, 0, 1 };
std::array<double, 4> values = { -1, 1, 1, 1 };

Highs h;
h.setOptionValue("output_flag", false);
h.setOptionValue("threads", 20); // setting to large number makes it more likely to deadlock

h.addVars(2, lb.data(), ub.data());
h.changeColsCost(2, index.data(), value.data());
h.addRows(2, lower.data(), ub.data(), 4, starts.data(), indices.data(), values.data());

std::cout << "starting" << std::endl;

for (int i = 0; i < 100; i++) {
   std::thread([&] { h.run(); }).join();
   std::cout << i << std::endl;
}

std::cout << "Done" << std::endl;
galabovaa commented 2 months ago

Closing, changes are now in https://github.com/ERGO-Code/HiGHS/pull/1948