cameron314 / concurrentqueue

A fast multi-producer, multi-consumer lock-free concurrent queue for C++11
Other
9.53k stars 1.66k forks source link

mainHash != nullptr #305

Closed jchen706 closed 2 years ago

jchen706 commented 2 years ago

I created the queue by

struct MyTraits : public moodycamel::ConcurrentQueueDefaultTraits { static const size_t BLOCK_SIZE = 256; // Use bigger blocks };

moodycamel::ConcurrentQueue<struct kernel*, MyTraits> kernelQueue;

I enqueue with

kernelQueue.enqueue(kernel) // which kernel is type struct kernel *

I get the below error:

/concurrentqueue.h:3408: moodycamel::ConcurrentQueue<T, Traits>::ImplicitProducer moodycamel::ConcurrentQueue<T, Traits>::get_or_add_implicit_producer() [with T = kernel; Traits = MyTraits]: Assertion `mainHash != nullptr' failed.

image

cameron314 commented 2 years ago

Can you provide a sample to reproduce? It works on my end.

mainHash is initialized from implicitProducerHash which is assigned in populate_initial_implicit_producer_hash from the constructor of ConcurrentQueue, so it cannot be null to begin with, though it does get re-assigned in a few places (should still never be null though).

jchen706 commented 2 years ago
#include <atomic> 
#include "concurrentqueue.h"
#include "pthread.h"

struct MyTraits : public moodycamel::ConcurrentQueueDefaultTraits
{
    static const size_t BLOCK_SIZE = 256;       // Use bigger blocks
};

struct scheduler {
    moodycamel::ConcurrentQueue<struct kernel*, MyTraits>  kernelQueue;
};

struct kernel {
  void** x; 
  std::atomic<int> count;

};

static scheduler* s;

int enqueue(struct kernel** k){
  s->kernelQueue.enqueue(*k);
}

int main ()
{
struct kernel* k = (struct kernel*)calloc(1,sizeof(struct kernel));
k->count++;

s = (scheduler *)calloc(1, sizeof(scheduler));
enqueue(&k);

}

I ran with g++ -Wall main.cpp -lpthread ./a.out

cameron314 commented 2 years ago

Well, that would do it :-) You're not calling the ConcurrentQueue constructor when you use calloc to allocate memory for the scheduler. Use s = new scheduler() instead.

(I think technically it's also undefined behaviour to bypass the constructor of std::atomic<int> but that will work by coincidence here).

jchen706 commented 2 years ago
#include <atomic> 
#include "concurrentqueue.h"
#include "pthread.h"

struct MyTraits : public moodycamel::ConcurrentQueueDefaultTraits
{
    static const size_t BLOCK_SIZE = 256;       // Use bigger blocks
};

struct scheduler {
    moodycamel::ConcurrentQueue<struct kernel*, MyTraits>  kernelQueue;
};

struct kernel {
  void** x; 
  std::atomic<int> count{0};

};

static scheduler* s;

int enqueue(struct kernel** k){
  s->kernelQueue.enqueue(*k);
}

int main ()
{
struct kernel* k = new struct  kernel;

int newTarget = 16;

bool exchanged = k->count.compare_exchange_strong(newTarget, newTarget);

int kcount = k->count;
printf("Exchanged: %d , k: %d \n", exchanged, kcount);

}

so exchange is always false, and the k->count is never going from 0 to 16. I am confused about undefined behavior to bypass the constructor of std::atomic, what does this mean?

cameron314 commented 2 years ago

Look up "undefined behaviour in C++". One thing which invokes undefined behaviour is bypassing the constructor of a (non-POD) object.