emptying an std::queue through a function that returns the front() and pops()

Question

I'm writing a template class that is supposed to provide a queue protected against concurrent access, basically to be able to write classes that can treat a list of items with a variable number of concurrent worker threads.

The template class has a method:

bool getFront(T &value)
{ CritSectEx::Scope scope(listLock);
  bool ret = false;
    if( !itemList.empty() ){
        value = itemList.front();
        itemList.pop();
        ret = true;
    }
    return ret;
}

which locks the queue, and if it's not empty fetches the 1st element, pops it and returns true, so that each worker can do

    while( getFront(entry) ){
        // do something with entry
        nTreated += 1;
    }
    // exit worker

The question is: what exactly am I returning here through the value reference? When I test this code, I get a double free error because the type T I'm using contains a pointer (let's call it B) that's deleted in its dtor (when B is not marked as being a pointer to a global structure). That dtor has been called during the pop() in getFront(), which seems logical. I took care that type T's ctors all allocate a copy of B when creating a new T from an existing instance and it does not subclass anything which could provide an '=' operator, so I'm not sure how I'd end up with 2 instances of T that each contain the same value of B and the "we own B" member.

I don't do this sort of thing often enough, so I'm probably overlooking something here, but what? Any suggestions how to troubleshoot this?

Additional observation: it appears that it's always the last element popped from the queue that shows this behaviour.

To make this less vague, here's what I use for typename T:

struct folder_info
{
    // plenty of members snipped
#ifdef __cplusplus
public:
    folder_info()
    {
        memset( this, 0, sizeof(struct folder_info) );
    }
    folder_info(const struct folder_info *src)
    {
        init(src);
    }
    folder_info(const struct folder_info &src)
    {
        init(&src);
    }
private:
    void init(const struct folder_info *src)
    {
        memcpy( this, src, sizeof(struct folder_info) );
        // we don't duplicate the filetypeslist!
        filetypeslist = NULL;
        filetypeslistlen = filetypeslistsize = 0;
    }
#endif
};

typedef struct FileEntry {
public:
    std::string fileName;
    struct stat fileInfo;
    FolderInfo *folderInfo;
    bool freeFolderInfo;
    FileEntry();
    FileEntry( const char *name, const struct stat *finfo, FolderInfo *dinfo, const bool ownInfo=false );
    FileEntry( const char *name, const struct stat *finfo, FolderInfo &dinfo );
    FileEntry(const FileEntry &ref);
    ~FileEntry();
} FileEntry;

FileEntry::FileEntry()
{
    folderInfo = NULL;
    freeFolderInfo = false;
}

FileEntry::FileEntry( const char *name, const struct stat *finfo, FolderInfo *dinfo, const bool ownInfo )
{
    fileName = name;
    fileInfo = *finfo;
    folderInfo = (ownInfo)? new FolderInfo(dinfo) : dinfo;
    freeFolderInfo = ownInfo;
}

FileEntry::FileEntry( const char *name, const struct stat *finfo, FolderInfo &dinfo )
{
    // creating a FileEntry with a FolderInfo reference always makes a copy
    // of the FolderInfo structure
    FileEntry( name, finfo, new FolderInfo(dinfo), true );
}

FileEntry::FileEntry(const FileEntry &ref)
{
    fileName = ref.fileName;
    fileInfo = ref.fileInfo;
    if( ref.freeFolderInfo ){
        folderInfo = new FolderInfo(ref.folderInfo);
    }
    else{
        folderInfo = ref.folderInfo;
    }
    freeFolderInfo = ref.freeFolderInfo;
}

FileEntry::~FileEntry()
{
    if( freeFolderInfo && folderInfo ){
        delete folderInfo;
        folderInfo = NULL;
    }
}

Answer 1

In case I get it right, the problem is the lack of a user-defined assignment operator for FileEntry.

Your copy constructor takes care of the fact that each FileEntry has a new FolderInfo if the copied FileEntry has freeFolderInfo == true, whereas your assignment operator does not.

If you assign one FileEntry to another one and if both have freeFolderInfo == true, you'll delete the same pointer twice, as soon as the last of the two goes out of scope / is deleted.

What is The Rule of Three?

If you need to explicitly declare either the destructor, copy constructor or copy assignment operator yourself, you probably need to explicitly declare all three of them.

Answer 2

"The question is: what exactly am I returning here through the value reference?"

Well if the output parameter isn't changed, the caller is primarily responsible for the value passed in. Though it might be better, to assign an appropriate default value before anything is done with it:

bool getFront(T &value)
{ 
  value = T(); // <<<<< Reset the output
//^^^^^^^^^^^^
  CritSectEx::Scope scope(listLock);
  bool ret = false;
    if( !itemList.empty() ){
        value = itemList.front();
        itemList.pop();
        ret = true;
    }
    return ret;
}

For pointers, this will result in a nullptr set to value, which won't cause a problem with delete.

Your double delete problems are most probably because the output value wasn't properly initialized/reset before calling getFront().

---

In general I would recommend to use smart pointers instead of raw pointers, and trying to manage dynamic memory allocation and deletion correctly on your own.

Using something like a type as std::unique_ptr<T> to be stored in your queue should solve your problems smoothly.