C moodycamel concurrent queue - enqueue pointers-CodePudding

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?

CodePudding user response：

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));