cxx_tls

[DenisYaroshevskiy]

I need C++ style thread locals: init on first access and then invoke a destructor.

[DenisYaroshevskiy]

Fails with copy elision issue. I will need to ask for herp here - thread locals in my case rely on the stability of their address

UPD: ah - that's just C++11. I will only do no copy in 17 or above

[dvyukov]

How is this different from just declaring a local variable here? It seems that it will have the same runtime behavior -- created here, destroyed at thread end...

[DenisYaroshevskiy]

You might be right actually, I will try to make that work.

I can't really do thread local statics with that:

void foo() {
  ThreadLocalWrapper x([]{ /*init*/});
}

but i was planning on doing that outside of tests anyways.

I will try and close the pr if it works.

QUESTION: why are other thread locas exist?

[dvyukov]

Good question.
I think the main reason would be to make it possible to convert existing code more easily (maybe with a layer of macros). Does it apply in this case?

[DenisYaroshevskiy]

I don't think so but let me get back to you on that after I try it this weekend

[DenisYaroshevskiy]

so - seems to work to just a use a local variable.

Some thing to thing about though:

• the ergonomics, discussed above.
• thread locals, in my case, heavily involve in global synchronization, they link themselves through a global list and such. I don't exactly understand how the library choses to order / swap execution but tls consturction / destruction could be important things
• the "access to a local variable that was already destroyed" is something I am greately concerned with in this case.

Any thoughts on this? Happy to close the pr otherwise.

[dvyukov]

the "access to a local variable that was already destroyed" is something I am greately concerned with in this case.

If the variable is created in the main thread function, then it should not happen.

But thinking more about this, I don't have strong opinion either way. Since relacy provides support for other types of TLS variables, providing support for this one looks reasonable and consistent.

@ccotter as an active recent contributor, do you have any options on if we should merge this or not.

[ccotter]

@DenisYaroshevskiy when you tried a local variable, you mean you tried a local variable within the thread() function that was of type rl::thread_local_var<complex_init> correct? How did you invoke the ctor (factory)?

[DenisYaroshevskiy]

No - the rl::thread_local ... is on the struct level.

With this pattern I did:

  Domain domain;
  rl::cxx_thread_local_var<tls_type> tls_storage;

  tls_type& tls(rl::debug_info_param info DEFAULTED_DEBUG_INFO) {
    return tls_storage.get([&]() { return tls_type{&domain}; });
  }

  ...
  
    auto& tls = this->tls();
    tls.enter();

With @dvyukov suggestion I did:

  Domain domain;

  tls_type make_tls(rl::debug_info_param info DEFAULTED_DEBUG_INFO) {
    rl::ctx().exec_log_msg(info, "make_tls");
    return tls_type{&domain};
  }
  
    auto tls = this->make_tls();
    tls.enter();

Full code here: https://github.com/DenisYaroshevskiy/concurency_experiments/blob/87936fc8add77d3627290ac30ed3283c0a45bde9/rl_tests/rcu_0_test.cpp#L29

The second option works but it's:

a) not as ergonomic (you have to pass it down to functions from thread)
b) the library doesn't get to control construction destruction as precisely
c) "access after the variable is destroyed" can't be caught by the library.

^ Note - all of these obviously apply to the local variables in the thread function

[ccotter]

should this be index == 2? the test is declared as rl::test_suite<tls_cxx_test, 3> (three total threads).

[ccotter]

I think the idea of a thread local with ctor/dtor (akin to how thread_local works for non trivial types) makes sense in Relacy.

I was thinking about the access pattern. With thread_local complex_init foo in standard (non-Relacy) code, we know exactly when the constructor runs - it's exactly where the thread_local var is declared. The access pattern here though is different, since the factor (constructor) is pased in both x.get() calls. The second x.get() call won't invoke the factory from what I understand. I wonder if we could change the interface so there is a single place where the factory/ctor is invoked (and maybe assert if it's invoked twice)?

[DenisYaroshevskiy]

The second x.get() call won't invoke the factory from what I understand.

Yep. This is the same as:

void foo() {
  static thread_local complex_init x = []{ }();
}

I wonder if we could change the interface so there is a single place where the factory/ctor is invoked.

The reason why I did it like this, thread_locals sometimes used in combination with local variables.
An tls.init([]{}) could work too, if that's better, no problem.

[ccotter]

actually, I don't think my suggestion makes sense, since when the ctor is called depends on the first use of the variable, which can depend on which line first accesses the thread_local.

thread_local complex_init a;

// if f1 is called first, the ctor runs then on .access().
// or, if f2 is called first, the ctor will run then
void f1() { a.access(); }
void f2() { a.access(); }

The standard only requires the ctor to be called before the first access; some implementations indeed call the ctor right upon first access, but others call the ctor when the thread is created.

In any case, perhaps a way to mimic c++11 thread_local more closely would be to have rl:: thread_local_var be constructed with the init function, and on first access, invoke the ctor. Is that doable? If not, then perhaps leave it as is.

That said, one caveat is that thread_local is also constructed if the address of the variable is taken, though I'm not sure if we can implement that with an overridden address-of operator (is that worth it - maybe not)?

[ccotter]

My only other comment is (given, I am a contributor but by no means a maintainer) lately, I've been adding new tests to separate .cpp / executables. This allows running the tests in parallel (ctest -j 4 for example). If you are up for it, moving the new tests and even other existing thread local tests out (now, or in a future PR) would be nice :)

[DenisYaroshevskiy]

I figured - let's just not - too much effort and the local variables work.