2

I create an on-heap array in class A and then create a thread_local A object. Accessing on-heap storage via A causes a segfault, why?

#include <chrono>
#include <iostream>
#include <memory>
#include <thread>
#include <vector>

class A {
 public:
  A() {
    std::cout << ">>>> constructor >>>>" << std::endl;
    a = new int[10];
  }
  ~A() {
    std::cout << ">>>> destructor >>>>" << std::endl;
    delete[] a;
  }
  void set(int x) { a[0] = x; }
  int get() const { return a[0]; }

 private:
  int* a;
};

int main() {
  std::thread t1;
  {
    thread_local A t;
    t1 = std::thread([]() {
      t.set(1);
      std::cout << t.get() << std::endl;
    });
  }

  t1.join();
  std::cout << ">>>> main >>>>" << std::endl;
}

Result:

>>>> constructor >>>>
Segmentation fault (core dumped)
coordinate
  • 99
  • 5
  • `int main() { A a1; A a2 = a1; }` you need to fix this double delete error if you intend to use `A` in a non-trivial program. Whether this is directly related to your problem or not, your `A` class violates the [rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). – PaulMcKenzie Apr 10 '22 at 04:07
  • An interesting thing about this program: if you do a `printf("t=%p\n", &t);` inside main(), and another `printf("t=%p\n", &t);` inside the thread-lambda-function, you'll notice that the two addresses printed out are different... AFAICT no constructor is ever called for the object at the second address, which is why when you call `t.set(1)`, the `set()` method tries to use an uninitialized pointer and crashes. I suspect that the problem is that you're trying to access a thread_local object from the wrong thread, but I don't understand the thread_local mechanism well enough to say for sure. – Jeremy Friesner Apr 10 '22 at 04:20
  • 3
    There are two instances of `A` in the program, one for each thread. Both are static local variables, initialized when execution first passes through the line where the object is declared. You managed to use one of those objects - the one that belongs to the worker thread - without the execution in that thread passing through its declaration. The program then exhibits undefined behavior by way of accessing an object before its lifetime has started. – Igor Tandetnik Apr 10 '22 at 04:24
  • @IgorTandetnik Is it specific to user defined types? in [this answer](https://stackoverflow.com/a/55213498/583833) theres a similar pattern with an int, and it seems to be initialized to 1 in all threads. I had a suspicion it was something like that (since the constructor print only happens once but theres two threads) but this answer made me doubt my logic. – Borgleader Apr 10 '22 at 04:28
  • It's specific to objects not initialized via zero- or [constant initialization](https://en.cppreference.com/w/cpp/language/constant_initialization). Anyway, that example is different - the worker thread uses the object that belongs to the main thread (the pointer to which is passed to the thread), not its own instance of the object. – Igor Tandetnik Apr 10 '22 at 04:35
  • @IgorTandetnik I was referring to the example with `void foo() { thread_local int n = 1;`, but i believe that is covered by the constant initialization part of your comment. Thanks. – Borgleader Apr 10 '22 at 04:37
  • Doesn't your lambda capture `t` by value? Making a copy is obviously fatal, since `A` is broken. Anyhow, run the code in a debugger to find out where exactly it fails. – Ulrich Eckhardt Apr 10 '22 at 06:17
  • @UlrichEckhardt "*Doesn't your lambda capture `t` by value?*" - no, because `t` is not specified in the lambda's capture list. – Remy Lebeau Apr 10 '22 at 06:38

1 Answers1

4

t is only initialised in when your code passes through it in main. As t1 is a separate thread its t is a separate instance of the variable but it is not initialised as the code in t1 never executes the line where t is declared.

If we change t to a global then the rules are different and each thread automatically initialises it's own copy of t and your code executes correctly:

thread_local A t;

int main() {
  std::thread t1;
  {
    t1 = std::thread([]() {
      t.set(1);
      std::cout << t.get() << std::endl;
      });
  }

  t1.join();
  std::cout << ">>>> main >>>>" << std::endl;
}

Unrelated to your problem but you should make sure that your code obeys the rule of three and either delete or implement the copy constructor and assignment operator otherwise this code will have undefined behaviour as both objects will try to delete a:

A a1;
A a2 = a1;
Alan Birtles
  • 32,622
  • 4
  • 31
  • 60
  • `As t1 is a separate thread its t is a separate instance of the variable` - how is this separate instance created? Non of constructor and/or assignment operator is called even if they were there. – YK1 Apr 10 '22 at 07:35
  • 1
    @YK1 its undefined behaviour, the memory for `t` is allocated but no initialiser is called, from some experimentation it does seem to be zero-initialised but I can't immediately find a reference to say that is required by the standard. – Alan Birtles Apr 10 '22 at 07:43
  • When I change `thread_local` to `static`, there is no problem, which is inconsistent with the statement in the article https://en.cppreference.com/w/cpp/language/storage_duration#Static_local_variables – coordinate Apr 10 '22 at 12:03
  • 1
    No, static means there's only one copy of the variable so it will always get initialised in `main` then the thread can use it – Alan Birtles Apr 10 '22 at 12:16
  • I have a new question about this. https://stackoverflow.com/questions/71828253/why-is-non-local-thread-local-not-constructed-in-main-thread – coordinate Apr 11 '22 at 13:04