idaholab / moose

Multiphysics Object Oriented Simulation Environment
https://www.mooseframework.org
GNU Lesser General Public License v2.1
1.76k stars 1.05k forks source link

Protect thread-unsafe routines using the libmesh Threads::in_threads boolean #23819

Open GiudGiud opened 1 year ago

GiudGiud commented 1 year ago

Reason

I recently ran into a FV routine that is not thread safe, because it was meant to be used in non-threaded region. We could communicate that intent, often well know from the developer of the routine, in an assertion, using this boolean that lets us know if threads are in use.

It's quite easy to add a lock if the routine starts being needed for threaded regions

Design

Assertions in thread-unsafe routines

mooseAssert(!Threads::in_threads, "This routine does not support threads currently. Please add a scoped_locked or call this outside of a threaded region")

Impact

Better development experience

GiudGiud commented 1 year ago

Roy had an insightful comment in a PR of mine related to this, I ll just paste here:

Originally in_threads was just intended to catch ourselves if we tried to nest a threaded loop inside another threaded loop, but it's a good idea for catching ourselves with any other code that shouldn't be called from inside threads ... with the limitation that it's only set to true by libMesh threads themselves. There's nothing we can do to prevent app level code from doing its own threading outside the libMesh APIs, but we might want to add a wrapper for anything likely (the C++17 parallel execution policy stuff?) before people start doing clever things with it, if we want to be able to extend enforcement to that too.