OsiClpSolverInterface::modelCut() is not threadsafe

[tosttost]

In line 3262

static int debugMode = 0;

this global variable is then modified in line 3443. It is unclear to me when/whether this line is executed. If it is and another thread B is running modelCut it is definitely possible that debugModel was not initialized in line 3296, thread A sets debugMode to a value != 0 and thread B calls functions with an uninitialized this pointer (e.g. line 3454) causing undefined behaviour or a segmentation fault.

OsiClpSolverInterface::modelCut is used by Cbc.

[jjhforrest]

As the name suggests, this is only used for debugging - which would be done without threads - normally in a debugger when it would be set by debugger.

On 13/05/2021 18:18, tosttost wrote:

In line 3262

|static int debugMode = 0; |

this global variable is then modified in line 3443. It is unclear to me when/whether this line is executed. If it is and another thread B is running modelCut it is definitely possible that debugModel was not initialized in line 3296, thread A sets debugMode to a value != 0 and thread B calls functions with an uninitialized this pointer (e.g. line 3454) causing undefined behaviour or a segmentation fault.

OsiClpSolverInterface::modelCut is used by Cbc.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/coin-or/Clp/issues/191, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWJYHCLZALNVNXADO2ISGLTNQCU3ANCNFSM4425OXWQ.

[tosttost]

this is only used for debugging

I do not claim anything else. But I'm pretty sure the current implementation is not thread safe no matter if you are debugging or not.

debugMode = 0 does skip all the 'if' that modify 'debugModel'. Nevertheless debugMode is modified if modelPtr->numberColumns < 0 no matter what value it had before or if we are running a debug or a release build. After that happend, the 'if' are no longer skipped.

So unless

• modelCut() is only called in debug builds - seems unlikely to me
• or 'modelPtr->numberColumns < 0' never happens in release builds

the current implementation is not thread safe in release builds, although the bug will be quite difficult to trigger.

[jjhforrest]

On 14/05/2021 10:41, tosttost wrote:

So unless

• modelCut() is only called in debug builds - seems unlikely to me
• or 'modelPtr->numberColumns < 0' never happens in release builds

I think I can guarantee that modelPtr_->numberColumns() will always be positive. I don't wish to lose some of that debugging code, but I will take out the lines

     if (numberColumns < 0)
       debugMode = -numberColumns;

John Forrest

[tosttost]

Sounds like a reasonable fix.

[tkralphs]

Just to follow up, I don't think this ever happened, right?

[tkralphs]

Oops, sorry, looks like it was done in refactor (5b6a9a6d82ab04dc428802adfa808612dab9ff73), which is the right place for it. I'm going to close this for now, feel free to reopen if needed.