pythonccpythonpyobject

Issue with the dequeue function in the custom class


I am building a RingBuffer class for my module. However, I am facing some issues with the dequeue function. When I enqueue/dequeue integers, everything is fine. When I enqueue a list, the first dequeue operation returns an empty list instead of the actual values. When I enqueue a string, the first dequeue operation returns meaningless data that says:

''_builtins__'

and then if I continue to dequeue, or call any methods, it gives an error

Segmentation fault (core dumped)

Here is the enqueue/dequeue functions:

static PyObject *
RingBuffer_enqueue(RingBuffer *self, PyObject *args)
{
    // its only works with integers, solve that and make it work with any type
    PyObject *item = NULL;

    if (!PyArg_ParseTuple(args, "O", &item))
        return NULL;

    // if its full, overwrite the oldest item
    if (self->size == self->capacity)
    {
        Py_DECREF(self->items[self->head]);
        self->items[self->head] = item;
        self->head = (self->head + 1) % self->capacity;
    }
    else
    {
        self->items[self->tail] = item;
        self->tail = (self->tail + 1) % self->capacity;
        self->size++;
    }

    Py_RETURN_NONE;
}

static PyObject *
RingBuffer_dequeue(RingBuffer *self)
{
    if (self->size == 0)
        Py_RETURN_NONE;

    PyObject *item = self->items[self->head];
    self->items[self->head] = NULL;
    self->head = (self->head + 1) % self->capacity;
    self->size--;

    return item;
}

and here's the RingBuffer type

typedef struct
{
    PyObject_HEAD;
    long capacity;
    long size;
    long head;
    long tail;
    PyObject **items;
} RingBuffer;

what is the problem that I cannot see? thanks in advance


Solution

  • item is a borrowed reference. If you're going to keep a reference to the appended item, you need a new reference:

    if (!PyArg_ParseTuple(args, "O", &item))
        return NULL;
    Py_INCREF(item);
    

    Aside from that, you also forgot to update the tail when you add an element at full capacity. Plus, in case an element tries to interact with the queue on Py_DECREF, you should decref the old element once you're done updating the queue, instead of while you're still in the middle of messing with it:

    if (self->size == self->capacity)
    {
        PyObject *old_head = self->items[self->head];
        self->items[self->head] = item;
        assert(self->head == self->tail);
        self->head = self->tail = (self->head + 1) % self->capacity;
        Py_DECREF(old_head);
    }