18

The C++11 standard says about local static variable initialization that it is supposed to be thread safe (http://en.cppreference.com/w/cpp/language/storage_duration#Static_local_variables). My question concerns what exactly happens when a lambda is initialized as a static local variable?

Let's consider the following code:

#include <iostream>
#include <functional>

int doSomeWork(int input)
{
    static auto computeSum = [](int number)                                                                                                                                                                  
    {
      return 5 + number;
    };  
    return computeSum(input);
}

int main(int argc, char *argv[])
{
    int result = 0;
#pragma omp parallel
{
  int localResult = 0;
#pragma omp for
  for(size_t i=0;i<5000;i++)
  {
   localResult += doSomeWork(i);
  }
#pragma omp critical
{
   result += localResult;
}
}

std::cout << "Result is: " << result << std::endl;

return 0;
}

compiled with GCC 5.4, using ThreadSanitizer:

gcc -std=c++11 -fsanitize=thread -fopenmp -g main.cpp -o main -lstdc++

Works fine, ThreadSanitizer gives no errors. Now, if I change the line where the lambda "computeSum" is initialized to this:

static std::function<int(int)> computeSum = [](int number)
{
  return 5 + number;
};  

The code still compiles, but ThreadSanitizer gives me a warning, saying there is a data race:

WARNING: ThreadSanitizer: data race (pid=20887)
  Read of size 8 at 0x000000602830 by thread T3:
    #0 std::_Function_base::_M_empty() const /usr/local/gcc-5.4_nofutex/include/c++/5.4.0/functional:1834 (main+0x0000004019ec)
    #1 std::function<int (int)>::operator()(int) const /usr/local/gcc-5.4_nofutex/include/c++/5.4.0/functional:2265 (main+0x000000401aa3)
    #2 doSomeWork(int) /home/laszlo/test/main.cpp:13 (main+0x000000401242)
    #3 main._omp_fn.0 /home/laszlo/test/main.cpp:25 (main+0x000000401886)
    #4 gomp_thread_start ../../../gcc-5.4.0/libgomp/team.c:118 (libgomp.so.1+0x00000000e615)

  Previous write of size 8 at 0x000000602830 by thread T1:
    #0 std::_Function_base::_Function_base() /usr/local/gcc-5.4_nofutex/include/c++/5.4.0/functional:1825 (main+0x000000401947)
    #1 function<doSomeWork(int)::<lambda(int)>, void, void> /usr/local/gcc-5.4_nofutex/include/c++/5.4.0/functional:2248 (main+0x000000401374)
    #2 doSomeWork(int) /home/laszlo/test/main.cpp:12 (main+0x000000401211)
    #3 main._omp_fn.0 /home/laszlo/test/main.cpp:25 (main+0x000000401886)
    #4 gomp_thread_start ../../../gcc-5.4.0/libgomp/team.c:118 (libgomp.so.1+0x00000000e615)

  Location is global 'doSomeWork(int)::computeSum' of size 32 at 0x000000602820 (main+0x000000602830)

  Thread T3 (tid=20891, running) created by main thread at:
    #0 pthread_create ../../../../gcc-5.4.0/libsanitizer/tsan/tsan_interceptors.cc:895 (libtsan.so.0+0x000000026704)
    #1 gomp_team_start ../../../gcc-5.4.0/libgomp/team.c:796 (libgomp.so.1+0x00000000eb5e)
    #2 __libc_start_main <null> (libc.so.6+0x00000002082f)

  Thread T1 (tid=20889, running) created by main thread at:
    #0 pthread_create ../../../../gcc-5.4.0/libsanitizer/tsan/tsan_interceptors.cc:895 (libtsan.so.0+0x000000026704)
    #1 gomp_team_start ../../../gcc-5.4.0/libgomp/team.c:796 (libgomp.so.1+0x00000000eb5e)
    #2 __libc_start_main <null> (libc.so.6+0x00000002082f)

SUMMARY: ThreadSanitizer: data race /usr/local/gcc-5.4_nofutex/include/c++/5.4.0/functional:1834 std::_Function_base::_M_empty() const

In any case, the code where ThreadSanitizer reports a data race needs to be executed 5-10 times until the warning mesage appears.

So my question is: is there a conceptional difference between

static auto computeSum = [](int number){ reentrant code returing int };

and

static std::function<int(int)> computeSum = [](int number) {same code returning int};

What makes the first code to work and the second to be a data race?

Edit #1: It seems that there was (is) quite a discussion going on about my question. I found Sebastian Redl's contribution the most helpful, thus I accepted that answer. I just want to summarize, so that people can refer to this. (Please le me know if this is not appropriate on Stack Overflow, I do not really ask stuff here...)

Why is a data race reported?

It was suggested in a comment (by MikeMB) that the problem is related to a bug in the gcc implementation of TSAN (see this and this link). It seems to be correct:

If I compile the code that contains:

static std::function<int(int)> computeSum = [](int number){ ... return int;};

with GCC 5.4, the machine code looks like:

  static std::function<int(int)> computeSum = [](int number)
  {
    return 5 + number;
  };
  4011d5:       bb 08 28 60 00          mov    $0x602808,%ebx
  4011da:       48 89 df                mov    %rbx,%rdi
  4011dd:       e8 de fd ff ff          callq  400fc0 <__tsan_read1@plt>
  ....

whereas, with GCC 6.3, it reads:

  static std::function<int(int)> computeSum = [](int number)                                                                                                                                             
  {
    return 5 + number;
  };
  4011e3:   be 02 00 00 00          mov    $0x2,%esi
  4011e8:   bf 60 28 60 00          mov    $0x602860,%edi
  4011ed:   e8 9e fd ff ff          callq  400f90 <__tsan_atomic8_load@plt>

I am not a big master of machine code, but it looks like that in the GCC 5.4 version, __tsan_read1@plt is used to check whether the static variable is initialized. In comparison, GCC 6.3 generates __tsan_atomic8_load@plt . I guess the second one is correct, the first one leads to a false positive.

If I compile the version without ThreadSanitizer, GCC 5.4 generates:

static std::function<int(int)> computeSum = [](int number)
{                                                                                                                                                                                                        
  return 5 + number;
};
400e17:     b8 88 24 60 00          mov    $0x602488,%eax
400e1c:     0f b6 00                movzbl (%rax),%eax
400e1f:     84 c0                   test   %al,%al
400e21:     75 4a                   jne    400e6d <doSomeWork(int)+0x64>
400e23:     bf 88 24 60 00          mov    $0x602488,%edi
400e28:     e8 83 fe ff ff          callq  400cb0 <__cxa_guard_acquire@plt>

And GCC 6.3:

  static std::function<int(int)> computeSum = [](int number)
  {                                                                                                                                                                                                      
    return 5 + number;
  };
  400e17:   0f b6 05 a2 16 20 00    movzbl 0x2016a2(%rip),%eax        # 6024c0 <guard variable for doSomeWork(int)::computeSum>
  400e1e:   84 c0                   test   %al,%al
  400e20:   0f 94 c0                sete   %al
  400e23:   84 c0                   test   %al,%al
  400e25:   74 4a                   je     400e71 <doSomeWork(int)+0x68>
  400e27:   bf c0 24 60 00          mov    $0x6024c0,%edi
  400e2c:   e8 7f fe ff ff          callq  400cb0 <__cxa_guard_acquire@plt>

Why is no data race, if I use auto instead of std::function?

You might have to correct me here, but probably the compiler "inlines" the auto object, so there is no need to do bookkeeping on whether the static object has been initialized or not.

static auto computeSum = [](int number){ ... return int;};

produces:

  static auto computeSum = [](int number)
  400e76:   55                      push   %rbp                                                                                                                                                          
  400e77:   48 89 e5                mov    %rsp,%rbp
  400e7a:   48 89 7d f8             mov    %rdi,-0x8(%rbp)
  400e7e:   89 75 f4                mov    %esi,-0xc(%rbp)
  //static std::function<int(int)> computeSum = [](int number)
  {
    return 5 + number;
  };
  400e81:   8b 45 f4                mov    -0xc(%rbp),%eax
  400e84:   83 c0 05                add    $0x5,%eax
  400e87:   5d                      pop    %rbp
  400e88:   c3                      retq
Community
  • 1
  • 1
user2583614
  • 193
  • 1
  • 6
  • Please do this experiment: define `int global = 100;` at global namespace, and then do `++global;` in the first lambda. See what the sanitizer says now. I believe it will give warning! – Nawaz Feb 06 '17 at 08:35
  • Seems to me as either a bug in threadsanitizer or in the implementation of std::function – MikeMB Feb 06 '17 at 08:40
  • @MikeMB: Quite likely! – Nawaz Feb 06 '17 at 08:41
  • Yes, it does give an error. I think this is not surprising, as in this case, the lambda _really_ accesses an unprotected shared resource, 'int global' . For your information, valgrind DRD complains about the second variant (with std::function) but does not complain about the first one. This either means that both DRD and TSAN is broken, or there is something weird going in with std::function. – user2583614 Feb 06 '17 at 08:41
  • I wont claim TSAN is broken. It could be that currently it doesn't go to that extent to prove there is no problem at all in your code. – Nawaz Feb 06 '17 at 09:01
  • @Nawaz: From your comments I don't know if this is clear to you (Sorry if I just misunderstood you and repeat things you know): TSAN doesn't prove the absence of a race and certainly won't throw an error just because it can't be 100% certain there isn't one. It will only output an error, if it honestly believes that there has been a race during the specific execution. – MikeMB Feb 06 '17 at 09:43
  • 1
    @MikeMB: I know about this post. They mention: "and looks like it fixed in trunk, but not in current stable release of gcc - 5.2" . I may falsely assumed that GCC 5.4 already contains the fix. Anyways, I am currently compiling GCC 6.3, and will report about what I found. – user2583614 Feb 06 '17 at 09:49
  • 1
    @user2583614: If I remember gcc's new versioning policy correctly, then all releases with the same major version number are on a single branch separate from trunk, that is split of at it's first release (e.g. Gcc5.1). All subsequent changes in the trunk will then only affect the next major release. Of course that doesn't mean, that fixes can't be backported. – MikeMB Feb 06 '17 at 09:58
  • @MikeMB: I was not aware of this. You are right: if I compile the code with GCC 6.3 and TSAN, no data race is reported. – user2583614 Feb 06 '17 at 13:42

3 Answers3

16

The C++ standard guarantees that initialization of local statics, no matter how complex, is thread-safe, in that the initialization code will run exactly once, and no thread will run past the initialization code before initialization is complete.

Furthermore, it guarantees that invoking a std::function is a read operation from the view of thread safety, meaning that an arbitrary number of threads may do it at the same time, as long as the std::function object is not modified at the same time.

By these guarantees, and because you code does not contain anything else that accesses shared state, it should be thread-safe. If it still triggers TSan, there's a bug somewhere:

  • Most likely, GCC is using very tricky atomic code for the static initialization guarantee, which TSan cannot identify as safe. In other words, it's a bug in TSan. Make sure you are using the most up-to-date versions of both tools that you can. (Specifically, it appears that TSan is somehow missing some kind of barrier that makes sure that the initialization of the std::function is actually visible to other threads.)
  • Less likely, GCC's initialization magic is actually incorrect, and there is a real race condition here. I cannot find any bug report to the effect that 5.4 had such a bug that was later fixed. But you might want to try with a newer version of GCC anyway. (Latest is 6.3.)
  • Some people have suggested that the constructor of std::function may have a bug where it accesses global shared way in an unsafe way. But even if this is true, it should not matter, because your code should not invoke the constructor more than once.
  • There may be a bug in GCC in inlining a function containing statics into an OpenMP parallel loop. Perhaps that leads to duplication of the statics, or breaking of the safe initialization code. Inspection of the generated assembly would be necessary for this.

The first version of the code is different, by the way, because it is completely trivial. Under -O3, GCC will actually completely compute the loop at compile-time, effectively converting your main function to

std::cout << "Result is: " << 12522500 << std::endl;

https://godbolt.org/g/JDRPQV

And even if it didn't do that, there is no initialization done for the lambda (the variable is just a single byte of padding), thus no write accesses to anything, and no opportunity for data races.

Sebastian Redl
  • 69,373
  • 8
  • 123
  • 157
5

The reasoning is wrong in both the answers posted so far.

It has nothing to do with lambda being function pointer. The reason is: if a function doesn't access unprotected shared data, then it is safe. In the case of auto computeSum= .. as defined in the question, which is simple, the ThreadSanitizer easily proves that it does not access any shared data. However, in case of std::function case, the code becomes a bit complex, and the sanitizer is either confused, or simply doesn't go to the extent to prove that it is still the same! It just gives up, seeing the std::function. Or it has bug — or worse, std::function is buggy!

Lets do this experiment: define int global = 100; at global namespace, and then do ++global; in the first lambda. See what the sanitizer says now. I believe it will give warning/error! That is enough to prove that it has nothing to do with lambda being function pointer as claimed by other answers.

As for your question:

Is initialization of a local static lambda thread safe?

Yes (since C++11). Please search this site for more detailed answers. This has been discussed many times.

Nawaz
  • 353,942
  • 115
  • 666
  • 851
  • 1
    1. I did the experiment. As expected, there is an error, because the lambda accesses an unprotected shared resource. – user2583614 Feb 06 '17 at 08:57
  • 2. I searched this site for answers but what I found were either about static local _variables_ (e.g. [this](http://stackoverflow.com/questions/8102125/is-local-static-variable-initialization-thread-safe-in-c11?rq=1) ) or static local lambdas that were also capturing something, e.g. [this](http://stackoverflow.com/questions/11619201/function-local-static-function-object-thats-initialized-by-lambda-thread-safe?rq=1). If I missed some answer, could you point me there? – user2583614 Feb 06 '17 at 09:01
  • @user2583614: Your so called lambda **is** a variable as well. Else what do you think it is? Not a variable? Lambda is nothing but object of some compiler generated class, with overloaded `operator()`. – Nawaz Feb 06 '17 at 09:04
  • I understand. Then the question that I asked was wrong. I wanted to know why the code above reported a data race, even though the standard suggests that it should not happen. Sorry about that. – user2583614 Feb 06 '17 at 09:09
  • @user2583614: My reasoning tells me that it is the fault of sanitizer or `std::function` has been implemented incorrectly, or both. I might be wrong... till someone comes up and proves it! – Nawaz Feb 06 '17 at 09:11
  • 1
    @Nawaz I think your answer is incorrect in that `std::function` cannot be at fault - if compiled correctly, the program should ever only execute the constructor once, in a single thread. Even if the constructor modified global shared state, this program would not trigger a race condition based on that. You'd need a different `std::function` object being initialized at the same time. – Sebastian Redl Feb 06 '17 at 09:38
  • @SebastianRedl: Theoretically you can write`std::function` in such a way that it can be at fault, even if there is no object of `std::function` being created at the same time.. and there could be other faulty ways as well. You cannot claim that *"`std::function` cannot be at fault"* unless you prove that it is *impossible* to implement it in a thread-unsafe way. And I think that would be a really tall claim! If you however meant in a probabilistic way, then I agree: the likelyhood is close to zero! – Nawaz Feb 06 '17 at 10:27
  • @Nawaz Well, it's open-source, so it's easy to check if it internally creates additional threads just for the sake of creating a race condition inside itself. – Sebastian Redl Feb 06 '17 at 10:31
  • @SebastianRedl: That is a different topic altogether. My argument is a *bit* theoretical. I agree with you probabilistically, though. – Nawaz Feb 06 '17 at 10:37
-1

Split the quest into two part:

First, is there a difference between lambda and closure during the static variable initialization ?

Yes, there is difference, but it doesn't matter, because gcc ensure the safety. and threadsanitizer may give warning because it isn't strong enough to analysis the std::function constructor.

The first one is lambda and the second one is closure.

The lambda contains a single field which is a function pointer and may not causing race problem because it is atomic.

But the closure is a function pointer with its enclosed variables, there are more than 1 fields in std::function, so can't be updated atomic.

Second, is there a difference between lambda and closure during the lambda/closure call ?

No, they are exactly the same. You function should protect the shared data used, no matter it is lambda or closure.

Zang MingJie
  • 5,164
  • 1
  • 14
  • 27
  • yes, the function variable is locked while initializing... both of them are locked. – Wagner Patriota Feb 06 '17 at 08:05
  • Non of the two cases is a closure the way you describe it(nothing is captured). Also static initialization is threadsafe, independent of the size of the object being initialized. So should be the function call operator on `std::function` – MikeMB Feb 06 '17 at 08:30
  • The reasoning is wrong in this answer. It has nothing to do with lambda being *function pointer*. The reason is: if a function doesn't access unprotected *shared* data, then it is safe. In the case of `auto lambda` = ..` as defined in the question which is simple, the ThreadSanitizer easily proves that it does not access any shared data. However, in case of `std::function` case, the code becomes a bit complex, and the sanitizer is confused, or doesn't go to the extent to prove that it is *still* the same! It just gives up, seeing the `std::function`. – Nawaz Feb 06 '17 at 08:37
  • @Nawaz there is nothing to do with the function, during the static variable initialization. The actually reason is *lambda is atomic and std::function is not*, my answer just points it out. – Zang MingJie Feb 06 '17 at 08:47
  • Lambda is "atomic" .. is still wrong reasoning. See my comment on the question. – Nawaz Feb 06 '17 at 08:48
  • @Nawaz you are totally wrong. Look the origin quest, The ThreadSanitizer throw a warning during the variable initialization, not during the lambda execution. So it doesn't matter what the lambda do. – Zang MingJie Feb 06 '17 at 08:52
  • @ZangMingJie: In C++11, initialization of `static` variable, even in function scope, is SAFE! The sanitizer is confused with the diagnostics. But your reasoning is wrong. Also, *pointer is atomic*, is also not true for all platform (anyway, that is the question here). – Nawaz Feb 06 '17 at 08:53
  • Also, the sanitizer gives warning when calling `std::function::operator()(int)`.. i.e during execution! (see the diagnostics again) – Nawaz Feb 06 '17 at 08:57
  • @ZangMingJie: 1) *"Yes, there is difference"* is not clear at all. 2)*"gcc ensure the safety."* , what safety? and is it GCC or the language which ensures it? – Nawaz Feb 06 '17 at 09:07
  • 2
    A lambda is not a function pointer. This is just plain wrong. – Sebastian Redl Feb 06 '17 at 09:10
  • @SebastianRedl I would expect the compiler generates a single function pointer for a lambda which capture nothing. I agree it won't be a function pointer if some variable captured. My answer refers *the lambda* which only means the lambda in the origin post – Zang MingJie Feb 06 '17 at 09:14
  • 1
    *"But the closure is a function pointer with its enclosed variables, there are more than 1 fields in std::function, so can't be updated atomic."*..... That is so wrong. Initialization of `static` objects in function scope, is thread-SAFE, since C++11. See [this](http://stackoverflow.com/questions/8102125/is-local-static-variable-initialization-thread-safe-in-c11). – Nawaz Feb 06 '17 at 09:16
  • @Nawaz what's wrong ? It can't be atomic but it can be thread-safe. There is no conflict between non-atomic and thread-safe. – Zang MingJie Feb 06 '17 at 09:19
  • 1
    @ZangMingJie: The lambda is not a function-pointer. It is compiler-generated class, with overloaded `operator()`. And in case, if it is non-capturing lambda, then it also provides an implicit conversion to function-pointer. But that does NOT mean lambda is function-pointer! – Nawaz Feb 06 '17 at 09:19
  • @Nawaz I havn't say A lambda is function pointer, I say THE lambda is function pointer. – Zang MingJie Feb 06 '17 at 09:20
  • 2
    @ZangMingJie No. It will create an empty class representing the lambda, with a call operator containing the code. The local variable is a single byte of padding, which is the representation of an empty class in all ABIs that I know of. The call to the lambda is a hard-coded direct call of the operator() code, with a pointer to the single byte being passed as the `this` pointer. Optimization will then probably inline the lambda call and get rid of everything, leaving just simple, straight-line code. (GCC optimizes the entire function to two machine instructions.) – Sebastian Redl Feb 06 '17 at 09:20
  • @ZangMingJie: So you're saying it is not atomic but thread-safe? If so, then how are you explaining the contrast you're trying to make with *"The lambda is a single function pointer which may not causing race problem because it is atomic"*. Please read your answer again... and the contrast you're creating, in the context of "data race". – Nawaz Feb 06 '17 at 09:22
  • @ZangMingJie: *THE lambda* is NOT function pointer. Why dont you search a bit more before you comment more? – Nawaz Feb 06 '17 at 09:25
  • @ZangMingJie: *"the lambda is atomic, and std::function isn't."* So? How is that even helpful here if you don't comment on whether the code is thread-safe or not? Also, On basis you're claiming *lambda is atomic*? It is an object of a compiler-generated class. – Nawaz Feb 06 '17 at 09:32