C++ moodycamel concurrent queue - enqueue pointers

Question

I am trying to use ConcurrentQueue to log items into a file on a separate thread:

https://github.com/KjellKod/Moody-Camel-s-concurrentqueue

This works:

// declared on the top of the file
moodycamel::ConcurrentQueue<MyType> q; // logger queue
. . .
int MyCallbacks::Event(MyType* p)
{
   MyType item = (MyType)*p;
   q.enqueue(item);
. . .
// pthread
void* Logger(void* arg) {

    MyType.item;

    while (true)
        if (!q.try_dequeue(item))

This doesnt (object appears to be corrupted after dequeue:

// declared on the top of the file
moodycamel::ConcurrentQueue<MyType*> q; // logger queue
. . .
int MyCallbacks::Event(MyType* p)
{
   MyType item = (MyType)*p;
   q.enqueue(&item);
. . .
// pthread
void* Logger(void* arg) {

    MyType* item;

    while (true)
        if (!q.try_dequeue(item))

also tried this in the Event - still doesnt work (the &newdata always prints same address):

auto newdata = std::move(data);
printf("  - pointers - new: %p old: %p\n", &newdata, &data);
q.enqueue(&newdata);

Am I doing it wrong or is it the queue doesnt support pointers?

If a template is written properly, it shouldn't matter what types you use. Pointers or non-pointers shouldn't matter, the behavior should be the same either way. — Some programmer dude, Mar 25 '22 at 15:13
With that said, what is your problem with the queue? What happens when you use it? What is supposed to happen? Please take some time to refresh [the help pages](http://stackoverflow.com/help), take the SO [tour], read [ask], as well as [this question checklist](https://codeblog.jonskeet.uk/2012/11/24/stack-overflow-question-checklist/). And don't forget how to create a [mre] or how to [edit] your questions. — Some programmer dude, Mar 25 '22 at 15:15
Oh, after you do `q.enqueue(&item);` do the life-time of `item` end before you dequeue the pointer? Will the `item` object be destructed before you try to use the pointer? If that happens, to what object is the pointer really pointing? Remember that copying a pointer only copies the actual pointer, not the object it points to. — Some programmer dude, Mar 25 '22 at 15:16
sorry I edited the code to be more realistic - i get pointer to item from 3rd party lib, I convert it to value and enqueue - works, if I pass pointer or if I try std::move and pass pointer - the dequeued object is corrupted. — Boppity Bop, Mar 25 '22 at 15:24
*Will the item object be destructed* - I dont know. This C++11 memory sorcery is all new to me. How do I tell compiler I want to destruct it manually ? — Boppity Bop, Mar 25 '22 at 15:35

Some programmer dude · Accepted Answer · 2022-03-25T16:02:51.200

The following code:

int MyCallbacks::Event(MyType* p)
{
   MyType item = (MyType)*p;
   q.enqueue(&item);

have a major flaw: You enqueue a pointer to the local variable item.

As soon as the Event function returns, the life-time of item ends, and it is destructed. The pointer to it that you saved will be invalid. Dereferencing that invalid pointer will lead to undefined behavior.

There's no need to create a local copy here at all, you should be able to work with p directly instead, including adding it to the queue:

q.enqueue(p);

With that said, you don't need the cast in

MyType item = (MyType)*p;

The type of *p already is MyType. Generally speaking, when you feel the need to do a C-style cast like that you should take it as a sign that you're doing something wrong.

If you need a copy, why not create a new object to copy-initialize?

Like:

MyItem* item = new MyItem(*p);
q.enqueue(item);

You of course have to remember to delete the object once you're finished with it.

A better solution than using raw non-owning pointers would be using a smart pointer like std::unique_ptr:

moodycamel::ConcurrentQueue<std::unique_ptr<MyType>> q; // logger queue

// ...

q.enqueue(std::make_unique<MyItem>(*p));

No I have to copy it somehow - the 3rd party lib is going to reuse memory as soon as I return from the callback. (I dont care if it is wrong - so far the "right" thing does not work for **me** - I switched from C/C++ 20 yrs ago and all the "right" things look to me a horrible nightmare). Can you help? it must be something very simple - copy an object and stuff it into a queue - should be dead simple. No? — Boppity Bop, Mar 25 '22 at 15:41

C++ moodycamel concurrent queue - enqueue pointers

1 Answers1