testFWCoreConcurrencyCatch2 failing in MULTIARCH_X, ROOT632_X, ROOT6_X

[iarspider]

In CMSSW_14_1_{MULTIARCH_X, ROOT632_X, ROOT6_X}, test testFWCoreConcurrencyCatch2 is failing:

src/FWCore/Concurrency/test/test_catch2_WaitingThreadPool.cc:118: FAILED:
  {Unknown expression after the reported line}
due to a fatal error condition:
  SIGABRT - Abort (abnormal termination) signal

[iarspider]

assign FWCore/Concurrency

[cmsbuild]

New categories assigned: core

@Dr15Jones,@makortel,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

[cmsbuild]

cms-bot internal usage

[cmsbuild]

A new Issue was created by @iarspider.

@smuzaffar, @rappoccio, @Dr15Jones, @sextonkennedy, @makortel, @antoniovilela can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[iarspider]

First occurance: CMSSW_14_1_ROOT632_X_2024-06-09-2300

[Dr15Jones]

The problem does not appear to be reproducible (as is true for many threading tests). Was the machine running the tests under a high load?

[Dr15Jones]

The actual failure in the log is

testFWCoreConcurrencyCatch2: src/FWCore/Utilities/interface/ReusableObjectHolder.h:92: edm::ReusableObjectHolder<T, Deleter>::~ReusableObjectHolder() [with T = edm::impl::WaitingThread; Deleter = std::default_delete<edm::impl::WaitingThread>]: Assertion `0 == m_outstandingObjects' failed.

[Dr15Jones]

I believe there is a race-condition in edm::asyncRun when the WaitingThreadPool is being destroyed, there is no guaranteed that the threads it spawned (via WaitingThread) have actually finished being returned to the ReusableObjectHolder so they can be properly deleted.

[iarspider]

Happened again in CMSSW_14_1_GEANT4_X_2024-07-07-2300:

===== Test "testFWCoreConcurrencyCatch2" ====
testFWCoreConcurrencyCatch2: src/FWCore/Utilities/interface/ReusableObjectHolder.h:92: edm::ReusableObjectHolder<T, Deleter>::~ReusableObjectHolder() [with T = edm::impl::WaitingThread; Deleter = std::default_delete<edm::impl::WaitingThread>]: Assertion `0 == m_outstandingObjects' failed.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
testFWCoreConcurrencyCatch2 is a Catch v2.13.6 host application.
Run with -? for options

-------------------------------------------------------------------------------
Test WaitingThreadPool
-------------------------------------------------------------------------------
src/FWCore/Concurrency/test/test_catch2_WaitingThreadPool.cc:14
...............................................................................

src/FWCore/Concurrency/test/test_catch2_WaitingThreadPool.cc:242: FAILED:
  {Unknown expression after the reported line}
due to a fatal error condition:
  SIGABRT - Abort (abnormal termination) signal

===============================================================================
test cases:  1 |  0 passed | 1 failed
assertions: 18 | 17 passed | 1 failed

timeout: the monitored command dumped core
/bin/sh: line 1: 3192312 Aborted                 timeout 3600 sh -c 'testFWCoreConcurrencyCatch2 '

---> test testFWCoreConcurrencyCatch2 had ERRORS
TestTime:1
^^^^ End Test testFWCoreConcurrencyCatch2 ^^^^

[makortel]

We took a deeper look with @Dr15Jones. The problem is likely the following: The code in

cmssw/FWCore/Concurrency/src/WaitingThreadPool.cc

Lines 47 to 55 in 94ac0f5

	func_();
	// Must return this WaitingThread to the ReusableObjectHolder in
	// the WaitingThreadPool before resettting func_ (that holds the
	// WaitingTaskWithArenaHolder, that enables the progress in the
	// TBB thread pool) in order to meet the requirement of
	// ReusableObjectHolder destructor that there are no outstanding
	// objects.
	thisPtr_.reset();
	decltype(func_)().swap(func_);

expects the thisPtr_.reset() (that returns the WaitingThread object to be returned to the ReusableObjectHolder to be called before the WaitingTaskWithArenaHolder::doneWaiting() in the swap() line (that then decrements the refcount of the WaitingTask, and may lead to the contained task to be scheduled if the refcount reaches 0). If the user-provided function throws an exception, the exception is communicated in

cmssw/FWCore/Concurrency/interface/WaitingThreadPool.h

Lines 33 to 38 in 94ac0f5

	try {
	convertException::wrap([&func]() { func(); });
	} catch (cms::Exception& e) {
	e.addContext(errorContext());
	holder.doneWaiting(std::current_exception());
	}

The problem is that the holder.doneWaiting(std::current_exception()) leads to the refcount to be decreased already during

cmssw/FWCore/Concurrency/src/WaitingThreadPool.cc

Line 47 in 94ac0f5

func_();

that can cause the task to be scheduled before the WaitingThread is returned to the ReusableObjectHolder.

The fix would be to add a function presetTaskAsFailed() (that WaitingTaskHolder has) to WaitingTaskWithArenaHolder, and use that to communicate the exception instead of doneWaiting(), and leave the refcount decrement to be done by the WaitingTaskWithArenaHolder destructor.

[makortel]

Hopefully fixed by #45402