0

I have some code where a thread callback effectively generates some data and writes it to a queue to be consumed by another thread looking something like this

auto data_ptr = std::make_shared<DataFrame>();
data_queue_.write(std::move(data_ptr));

I know it was written this way as to avoid copies when reading and writing from the queue. I need to instantiate another thread to consume and do some work with this data. Will doing something like this even be logical

# Adding this
custom_queue_.write(std::move(data_ptr));

Does that mean that the object only gets deleted when it gets pulled out of both of the threads reading from this queue and then only the data_ptr memory allocated gets deleted?

raaj
  • 2,869
  • 4
  • 38
  • 58

1 Answers1

1

Does that mean that the object only gets deleted when it gets pulled out of both of the threads reading from this queue and then only the data_ptr memory allocated gets deleted?

No it doesn't. The first use of std::move will 'rob' data_ptr and the second is, effectively, UB.

Don't be afraid to copy a std::shared_ptr. It is a cheap operation and doesn't copy the underlying data. In fact, that is the whole idea behind std::shared_ptr - to share ownership of whatever it points to, with the object being deleted when the last shared_ptr goes out of scope.

If you're desparately worried about performace (which I am not) then you could pass data_ptr to data_queue_.write by value (thus making a copy) and to custom_queue_.write by reference (thus not making a copy). In neither case is std::move appropriate or useful.

But code like this is fragile. I strongly recommend you keep things simple and pass data_ptr by value in both cases. This is how shared_ptr is meant to be used. People don't talk about modern C++ having value semantics for nothing.

Paul Sanders
  • 24,133
  • 4
  • 26
  • 48
  • Note that you may still have `data_ptr` hanging out in scope somewhere keeping the object alive after the threads have ended. – user4581301 Jun 09 '21 at 18:56
  • @user4581301 Need to see more context to answer that one., but if it's existing code ten it's probably OK. – Paul Sanders Jun 09 '21 at 18:58
  • Yup. And the solutions (tighter scope, call to `reset`, ...) if it's a problem are plentiful and simple. – user4581301 Jun 09 '21 at 19:05
  • So i should copy the shared pointer first and do an std move of that copy to the second queue? When you say copy, you dont actually mean a deep copy but a soft copy (reference) yes? – raaj Jun 09 '21 at 20:38
  • No, just pass it by value to both threads, thus giving each thread a copy which goes out of scope when the thread ends. That is the concept of shared ownership of the pointed-to object that I was talking about. – Paul Sanders Jun 09 '21 at 20:39
  • I could, but it isn't code I wrote (the first queue). I am trying to rationalize why std::move was used then actually. – raaj Jun 09 '21 at 20:42
  • No reason, that I can see. With only one thread it is harmless (unless you subsequently want to use `data_ptr` in the main thread), but with two or more threads it's a problem. – Paul Sanders Jun 09 '21 at 20:44
  • https://stackoverflow.com/questions/41871115/why-would-i-stdmove-an-stdshared-ptr Moving a shared pointer is 100x more efficient? – raaj Jun 09 '21 at 20:55
  • Added a rider to my answer. Firstly, I really doubt that 100x is the real figure on modern architectures, and secondly the performance penalty, in real terms, is almost certainly insignificant anyway. – Paul Sanders Jun 09 '21 at 22:03