Troubleshooting memory leak when streaming serial device data to GTK text buffer

[arctagon]

I am building an application in Windows that can communicate with multiple serial devices. Sending and receiving data to a device is handled with the Win32 API and the GUI is handled by GTK3. A worker thread is created by g_thread_new to run the device polling loop and the text view/buffer is updated by using g_timeout_add. When the device is streaming, its output is in ASCII text which is then inserted into a buffer if it is determined that there is a new line that has not yet been inserted. The polling loop runs infinitely until a disconnection function is triggered which then cleans up the resources.

I have discovered a memory leak that I believe to be somewhere within the buffer update function. When this function is commented out (i.e. nothing gets inserted into the buffer), I no longer have a leak.

gboolean
updateViewport (gpointer data) {

SerialDevice *device = (SerialDevice *) data;
device = getDeviceById (device->id);

if (device != NULL) {

    if (device->dataStringBuffer[device->lineCount - 1] != NULL && device->newDataForDisplay == PENDING) {

        GtkTextMark *mark;
        GtkTextIter start, end;

        mark = gtk_text_buffer_get_insert ( device->viewPort->textViewBuffer);
        gtk_text_buffer_get_iter_at_mark ( device->viewPort->textViewBuffer, &iter, mark );
        gtk_text_buffer_get_bounds ( device->viewPort->textViewBuffer, &start, &end );
        gtk_text_buffer_get_iter_at_offset (device->viewPort->textViewBuffer, &iter, -1);

        if (device->viewPort->linesDisplayingInBuffer > lineLimit) {
            gtk_text_buffer_delete(device->viewPort->textViewBuffer, &start, &end);
            device->viewPort->linesDisplayingInBuffer = 0;
        }

        gtk_text_buffer_insert (device->viewPort->textViewBuffer, &iter, device->dataStringBuffer[device->lineCount - 1], -1);

        device->newDataForDisplay = EMPTY;
        device->viewPort->linesDisplayingInBuffer++;

        gtk_text_buffer_delete_mark(device->viewPort->textViewBuffer, mark);
    }
}

return G_SOURCE_CONTINUE;
}

For now, I want to limit the total number of lines that the user can scroll through. When the limit is reached, I am using the gtk_text_buffer_delete method to clear out the buffer. I am calling gtk_text_buffer_delete_mark to release the reference to the mark I created.

Incoming data read into incomingBuffer is copied into dataStringBuffer, an array of strings which is then inserted into the text view buffer. The memory for dataStringBuffer is allocated on device creation and released when the device is disconnected.

   if (bitsRead > 1) {
    // Reset lineCount to overwrite the lines that have already been displayed
    if (device->lineCount == DATA_STRING_BUFFER_SIZE - 1) {
        device->lineCount = 0;
    }

    g_strlcpy(device->dataStringBuffer[device->lineCount], device->incomingBuffer, INCOMING_BUFFER_SIZE);
    device->lineCount++;
    device->newDataForDisplay = PENDING;

    PurgeComm(device->hSerialComm, PURGE_RXCLEAR);
    memset(device->incomingBuffer, 0, INCOMING_BUFFER_SIZE);
}

Memory consumption is small when only a single device is streaming to a buffer, however this is not the norm. There will typically be 6 to 8 devices streaming at the same time, each on their own worker thread, which magnifies the impact of the leak. My hunch is that references are not being released when I am calling gtk_text_buffer_delete to delete the lines out of the buffer, however I’m new the mechanics GTK library.

Any advice/criticism/feedback is more than welcome.

[emergenz]

I think that your updateViewport function needs some corrections:

(1) gtk_text_buffer_delete_mark is not necessary because the insert mark is a constant one; you neither can delete it nor need to;

(2) you set a value to the variable iter with the function gtk_text_buffer_get_iter_at_mark, but then you do not use it anywhere in the function body and overwrite with a new value by calling the function gtk_text_buffer_get_iter_at_offset; however, the variable iter is a global one, so I can guess that you probably initialize it for some side effects? If not, I would say that you do not need the stuff related to the variable mark at all;

(3) after you have deleted the text with gtk_text_buffer_delete the text iterator saved in the variable iter becomes invalid, so if you need an end (or start) iterator then call its initialization after deleting.

We get then something like this:

gboolean updateViewport (gpointer data) {

SerialDevice *device = (SerialDevice *) data;
device = getDeviceById (device->id);

    if (device != NULL) {

        if (device->dataStringBuffer[device->lineCount - 1] != NULL && device->newDataForDisplay == PENDING) {

            if (device->viewPort->linesDisplayingInBuffer > lineLimit) {
                GtkTextIter start, end;
                
                gtk_text_buffer_get_bounds ( device->viewPort->textViewBuffer, &start, &end );              

                gtk_text_buffer_delete(device->viewPort->textViewBuffer, &start, &end);
                
                device->viewPort->linesDisplayingInBuffer = 0;

                gtk_text_buffer_get_iter_at_offset (device->viewPort->textViewBuffer, &iter, 0);
            }
            else gtk_text_buffer_get_iter_at_offset (device->viewPort->textViewBuffer, &iter, -1);

            gtk_text_buffer_insert (device->viewPort->textViewBuffer, &iter, device->dataStringBuffer[device->lineCount - 1], -1);

            device->newDataForDisplay = EMPTY;
            device->viewPort->linesDisplayingInBuffer++;
        }
    }

    return G_SOURCE_CONTINUE;
}

[lb90]

Also beware of the Cairo leak described in https://gitlab.gnome.org/GNOME/gtk/-/issues/5154

[lb90]

Are you using some package manager like e.g vcpkg or MSYS2?

[arctagon]

(1) gtk_text_buffer_delete_mark is not necessary because the insert mark is a constant one; you neither can delete it nor need to;

That makes sense. I don’t hold any other references to that mark in my code but when I read the docs for gtk_text_buffer_get_insert, I interpreted that as something that needed to be free’d before leaving the block.

however, the variable iter is a global one, so I can guess that you probably initialize it for some side effects?

Yes, it gets used in another section of the code when a user to sends a command to the device by using a GTK Entry. The command sent (in ASCII) is then inserted into the text buffer after the last streamed line to keep track of issued commands.

(3) after you have deleted the text with gtk_text_buffer_delete the text iterator saved in the variable iter becomes invalid, so if you need an end (or start) iterator then call its initialization after deleting.

I was able to get by without that because I was initializing iter in the function that sends commands to the device and inserts the sent command into the text buffer. Since I’m reusing iter, what you suggest makes sense after I call gtk_text_buffer_delete.

Aside from what you mentioned above, did you notice any areas that may be causing a leak?

[arctagon]

I’m experiencing a similar behavior to what I see happening with the Cario leak issue. I’m frequently inserting into the buffer (~100ms).

Are you using some package manager like e.g vcpkg or MSYS2?

Yes, MSYS2.

[lb90]

Then the issue could be fixed by upgrading with the pacman -Syu command. Note that you may need to run the command twice as described in Updating MSYS2 - MSYS2)

[arctagon]

I’ll try updating and get back to you!

UPDATE: After testing with multiple devices, the package update resolved my problem. I no longer have a leak
Thanks, for the support @lb90 @emergenz

[emergenz]

@arctagon

Aside from what you mentioned above, did you notice any areas that may be causing a leak?

Honestly said, I have no clear understanding of your program design. I would use in the body of updateViewport, if possible, local variables and use globals only for the communication between functions.