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?
The following code:
have a major flaw: You enqueue a pointer to the local variable
item
.As soon as the
Event
function returns, the life-time ofitem
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:With that said, you don't need the cast in
The type of
*p
already isMyType
. 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:
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
: