2

I am dealing with a large confusing codebase, and I have a situation like this. 

#pragma omp parallel for 
for ( int i = 0; i < N; i++) {
  problemFunction();
}

void problemFunction() {

  static bool inFunction;
  if ( inFunction == true ) {
    return;
  }
  else {
    inFunction = true;
  }
}

Would this create a race condition?

v2v1
  • 640
  • 8
  • 18
  • This does create a potential race condition if different threads set conflicting values in a static, it's a common source of parallelization bugs. – tim18 Feb 24 '18 at 03:29
  • why does the static variable create a race condition when a non-static one would not? you are saying the state of inFunction is shared across threads? – v2v1 Feb 24 '18 at 03:33
  • A local auto variable gives each thread an independent value, thus no race. – tim18 Feb 24 '18 at 03:37
  • I understand that. What I'm trying to ask, and don't understand, is why the static variable is not ALSO created for each thread. – v2v1 Feb 24 '18 at 03:39
  • 'Would this create a race conditon?" Yes. "Why the static variable is not ALSO created for each thread?" Uh, because it's not? A static variable is created and initialized at program startup. Static has nothing to do with threads. – Jive Dadson Feb 24 '18 at 05:18
  • The complete answer to your question is simply *"yes"*. Please clarify your question by editing it. – Zulan Feb 24 '18 at 09:04
  • @JiveDadson, there a one [connection between `static` and threads](https://stackoverflow.com/q/1661529/620382) (that does not apply to this question!). – Zulan Feb 24 '18 at 09:22
  • It does not create _race condition_ but _data race_. That's different. – Daniel Langr Feb 24 '18 at 09:48
  • The use of static, such as you quote, appears to be for the purpose of allowing the function to "remember" a setting. Unless you introduce a separate variable for each thread, there is no way (other than the thread_local proposed in an answer) for the function to pick up what was set previously. – tim18 Feb 24 '18 at 10:51

2 Answers2

3

Unlike automatic local variables that usually reside on the thread's stack (which is private), local variables with static storage class reside in the data segment of the process and are thus shared between all threads executing the given function, therefore your code contains a race condition. To prevent the sharing, you should use OpenMP's threadprivate construct:

void problemFunction() {
  static bool inFunction;
  #pragma omp threadprivate(inFunction)

  if ( inFunction == true ) {
    return;
  }
  else {
    inFunction = true;
  }
}

This will alter the place the variable is stored in from the data segment to the so-called Thread-Local Storage (TLS). The original static semantics are kept, i.e. the value is retained between the function calls, but now each thread has its own copy. The threadprivate construct should always come after the variable declaration.

Hristo Iliev
  • 72,659
  • 12
  • 135
  • 186
0

Yes, this will cause a race at execution (obviously). To avoid, you can use thread_local (see also here) variable (also, your function seems unnecessary complicated):

void problemFunction() {
  static thread_local bool inFunction;
  inFunction = true;
}

when each thread has their own variable inFunction. This way your function works with any threading implementation, not just OpenMP.

Walter
  • 44,150
  • 20
  • 113
  • 196
  • 1
    I'll never get why people feel the need to say "obviously." Go ahead, laugh at me, I am new to this game. I am really tired of the general attitude on this website. – v2v1 Feb 24 '18 at 13:59
  • 1
    @xcski I said *(obviously)* since I don't provide further explanation. I was of the impression that someone who (1) uses `static` variables and (2) uses multi-threading knows enough about these topics to understand that and why this will be a data race. Perhaps I anticipated that you have read (& understood) some basic text on concurrency. I would recommend doing so *before* starting to code and/or asking on SO. – Walter Feb 24 '18 at 18:58
  • The question is clearly tagged as `[openmp]`. Why are you then proposing a solution that uses a language keyword instead of the OpenMP construct? There is no guarantee whatsoever that the OpenMP runtime will interoperate with the compiler's implementation of thread-local variables. Besides, `thread_local` is a C++11 keyword. – Hristo Iliev Feb 26 '18 at 23:48
  • @HristoIliev As I think we had discussed in the elsewhere on SO, there is no guarantee (by any standard) that OpenMP interoperates with C++ in any case. Using `thread_local` worked for me in a similar situation. C++11 is an (almost) 7 years old standard and widely considered the *least* to expect these days (and implied by the SO tag `C++`). – Walter Feb 27 '18 at 00:31