Synchronization error in consumer/producer style logger

Question

I see lots of this sort of questions but none helped me so I post mine.

I made a logger for multi-thread logging. Only one thread(Created when the very first logger is created.) deals with log messages from many threads and the buffer is synchronized using condition variables. One for push() and one for pop().

It seemed working fine but it doesn't work when it comes to too many messages in a short time.

Here's the exception message from xutility.

Exception thrown: read access violation.
_Pnext was 0xFFFFFFFFFFFFFFFF.

And this is my code.

logger.h

class ConsoleLogger
{
private:
    static Buffer<std::string, 1000> buffer;
    uint32_t curLine = 0;
    static bool instantiated;
    std::mutex logLock;
    std::string prefix;
    std::ostringstream logStream;

    // creates log thread one very first logger calls constructor
    void createLogThread();
    // keeps logging incoming logs
    static void logThreadFunc();

public:
    ConsoleLogger();
    void setPrefix(const std::string_view& newPrefix);

    template<typename T, typename... Types>
    void log(T log1, Types... logs) {
        {
            std::scoped_lock<std::mutex> lock(logLock);
            noLog = false;

            logStream.clear();
            logStream.str("");
            logStream << prefix << "#" << curLine << "\t" << std::this_thread::get_id() << "\t";
            ++curLine;

            logMessage(log1, logs...);
        }
    }

private:
    void logMessage() {
        logStream << "\n";

        std::string* str = new std::string{ logStream.str() };

        buffer.push(str);
    }

    template<typename T, typename... Types>
    void logMessage(T log1, Types... logs) {
        logStream << log1;
        logMessage(logs...);
    }
};

logger.cpp

Buffer<std::string, 1000> ConsoleLogger::buffer;

void ConsoleLogger::logThreadFunc() {
    while (true) {
        std::string* log = buffer.pop();
        if (log == nullptr) {
            std::cerr << "log was nullptr" << std::endl;
            continue;
        }
        std::cout << *log;
        delete log;          <-------------throws exception

    }
}

buffer.h

template<class T, uint64_t Qsize>
class Buffer
{
private:
    T* arrayPtr[Qsize];
    uint64_t head;
    uint64_t tail;
    static const uint64_t mask = Qsize - 1;

    std::condition_variable condEmpty;
    std::condition_variable condOverflow;
    std::mutex mtx;
public:
    Buffer();

    void push(T* x);
    T* pop();
};

template <class T, uint64_t Qsize>
Buffer<T, Qsize>::Buffer() : head(0),
                             tail(0)
{}

template <class T, uint64_t Qsize>
void Buffer<T, Qsize>::push(T* x)
{
    std::unique_lock<std::mutex> lock(mtx);

    condOverflow.wait(lock, [this]() {
        return tail + Qsize > head;
        });

    arrayPtr[head++ & mask] = x;

    lock.unlock();
    condEmpty.notify_one();
}

template <class T, uint64_t Qsize>
T* Buffer<T, Qsize>::pop()
{
    std::unique_lock<std::mutex> lock(mtx);

    condEmpty.wait(lock, [this]() {
        return tail < head;
        });

    T* x = arrayPtr[tail++ & mask];

    lock.unlock();
    condOverflow.notify_one();

    return x;
}

I've been trying to fix this for the last couple days but I have no idea why is this happening. I think it keeps trying to access already deleted memory. I tested without deleting and it logs the same message it already printed.

Any suggestions or help would be very thankful.

Answer 1

You should explicitly null out the elements of the arrayPtr

template <class T, uint64_t Qsize>
Buffer<T, Qsize>::Buffer() : head(0),
                             tail(0)
{
    for (uint64_t i = 0; i < Qsize; i++)
        arrayPtr[i] = nullptr;
}