2

I have a Python script that handles Modbus communications. One feature I added was a "graph" that shows the response times along with a color coded line that indicates if the response was successful, had an exception, or an error. The graph is just a scrollable canvas widget from Tkinter.

After graphing a certain number of lines old lines will be deleted and then a new one will be added to the end. For this example I have it set to 10, which means there will never be more than 10 lines on the canvas at once.

The code works correctly but there is a memory leak somewhere in this function. I let it run for about 24 hours and it took about 6x more memory after 24 hours. The function is part of a larger class.

My current guess is that my code causes the canvas size to constantly "expand," which slowly eats up the memory.

self.lineList = []
self.xPos = 0

def UpdateResponseTimeGraph(self):
    if not self.graphQueue.empty():
        temp = self.graphQueue.get() #pull from queue. A separate thread handles calculating the length and color of the line. 
        self.graphQueue.task_done()

        lineName     = temp[0] #assign queue values to variables
        lineLength   = temp[1]
        lineColor    = temp[2]

        if len(self.lineList) >= 10: #if more than 10 lines are on the graph, delete the first one.
            self.responseTimeCanvas.delete(self.lineList[0])
            del self.lineList[0]

        #Add line to canvas and a list so it can be referenced.
        self.lineList.append(self.responseTimeCanvas.create_rectangle(self.xPos, self.responseWidth, self.xPos + 4, self.responseWidth-lineLength, 
                                                fill=lineColor, outline=''))

        self.xPos += 5 #will cause the next line to start 5 pixels later. MEMORY LEAK HERE?

        self.responseTimeCanvas.config(scrollregion=self.responseTimeCanvas.bbox(ALL))

        self.responseTimeCanvas.xview_moveto(1.0) #move to the end of the canvas which is scrollable.



    self.graphFrame.after(10, self.UpdateResponseTimeGraph)

One solution could be loop back to the start of the graph once a limit is reached but I would rather not do this since it may be confusing where the graph starts. Usually I have far more responses than 10.

EDIT:

I'm still doing to trail and error stuff but it looks like the memory leak can be eliminated with Bryan's suggestion as long as the line attributes are not changed via itemconfig. The code below should be able to run as is, if you're on python 2.7 change the import statement from tkinter to Tkinter (lower case vs uppercase t). This code will have the memory leak in it. Comment out the itemconfig line and it will be eliminated.

import tkinter
from tkinter import Tk, Frame, Canvas, ALL
import random

def RGB(r, g, b):
    return '#{:02x}{:02x}{:02x}'.format(r, g, b)

class MainUI:
    def __init__(self, master):
        self.master = master
        self.lineList = []
        self.xPos = 0

        self.maxLine = 122

        self.responseIndex = 0 


        self.responseWidth = 100
        self.responseTimeCanvas = Canvas(self.master, height=self.responseWidth)
        self.responseTimeCanvas.pack()

        self.UpdateResponseTimeGraph()

    def UpdateResponseTimeGraph(self):
        self.lineLength   = random.randint(10,99)

        if len(self.lineList) >= self.maxLine:
            self.lineLength = random.randint(5,95)
            self.responseTimeCanvas.coords(self.lineList[self.responseIndex % self.maxLine], self.xPos, self.responseWidth, self.xPos + 4, self.responseWidth-self.lineLength)

            #if i comment out the line below the memory leak goes away.
            self.responseTimeCanvas.itemconfig(self.lineList[self.responseIndex % self.maxLine], fill=RGB(random.randint(0,255), random.randint(0,255), random.randint(0,255)))
        else:
            self.lineList.append(self.responseTimeCanvas.create_rectangle(self.xPos, self.responseWidth, self.xPos + 4, self.responseWidth-self.lineLength, 
                                                fill=RGB(random.randint(0,255), random.randint(0,255), random.randint(0,255)), outline=''))


        self.xPos += 5 #will cause the next line to start 5 pixels later. MEMORY LEAK HERE?
        self.responseIndex += 1

        self.responseTimeCanvas.config(scrollregion=self.responseTimeCanvas.bbox(ALL))

        self.responseTimeCanvas.xview_moveto(1.0) #move to the end of the canvas which is scrollable.



        self.responseTimeCanvas.after(10, self.UpdateResponseTimeGraph)


mw = Tk()
mainUI = MainUI(mw)
mw.mainloop()
Dave1551
  • 323
  • 2
  • 13
  • I don't see anything obviously wrong...but you could try changing the existing `Canvas` object in `lineList[0]` via the widget's `itemconfigure()` method and modifying the rectangle that already exists, instead of deleting it and creating a new one as you're now doing. – martineau Mar 07 '19 at 22:30
  • That's my plan B right now. I think it looks better when constantly adding to the end and it makes it more clear cut what order things happened. I have an older version of this script and the leak exists but doesn't cause problems unless I'm running it for multiple days while doing rapid reads. After that it will freeze up or crash. – Dave1551 Mar 07 '19 at 22:36
  • You can change the order of the existing items in a `Canvas` display list by calling the widget's `tag_Lower()` or `tag_raise()` methods. – martineau Mar 07 '19 at 22:40

2 Answers2

1

The underlying tk canvas doesn't reuse or recycle object identifiers. Whenever you create a new object, a new identifier is generated. The memory of these objects is never reclaimed.

Note: this is memory inside the embedded tcl interpreter, rather than memory managed by python.

The solution is to reconfigure old, no longer used elements rather than deleting them and creating new ones.

Bryan Oakley
  • 370,779
  • 53
  • 539
  • 685
  • I don't know the internals like you do, but it sounds like you're confirming that my suggestion would avoid the issue. – martineau Mar 07 '19 at 22:46
  • 1
    @martineau: I didn't see your suggestion until now, but yes, we agree on the solution. – Bryan Oakley Mar 07 '19 at 22:57
  • Thanks. @martineau I may have misinterpreted your suggestion. I changed my code to hopefully fix this issue. Rather than deleting the line when it reaches the limit I take the first line and place it at the end. For now I don't change any other attributes. This means the canvas is still 'expanding' but it looks like there is no longer a memory leak. I'll have to let it run for a few hours before I can be sure. I'll update this thread once it runs long enough – Dave1551 Mar 07 '19 at 23:10
  • @Dave1551: FWIW, that sounds basically correct as I don't _think_ not also having a call to `itemconfigure()` to change the object's attributes will matter - but again I don't know a lot about the internal details. – martineau Mar 07 '19 at 23:19
  • The memory leak went away if i don't use the itemconfig to change the lines' color. When just moving the line the memory started at 6.8MB, then went up to 7.2 pretty quickly, then eventually dropped to 2MB which I can't explain. When changing the color with itemcofig the memory slowly increases. I added standalone code to my original post if anyone want's to experiment with this too. – Dave1551 Mar 08 '19 at 14:55
  • @dave1551: Given that you're creating potentially 256*256*256 colors, that color information has to be stored somewhere. If you remove the random colors and pick from a fixed set of a couple dozen colors, does the memory still increase? – Bryan Oakley Mar 08 '19 at 17:05
  • @Dave1551 please add your edit as a new answer, thanks – Vickel Mar 09 '19 at 00:19
1

Here's the code with no memory leak. The original source of the leak was me deleting the old line then creating a new one. This solution moves the first the line to the end then change's its attributes as necessary. I had a second 'leak' in my example code where I was picking a random color each time which lead to the number of colors used eating up a lot of memory. This code just prints green lines but the length will be random.

import tkinter
from tkinter import Tk, Frame, Canvas, ALL
import random

def RGB(r, g, b):
    return '#{:02x}{:02x}{:02x}'.format(r, g, b)

class MainUI:
    def __init__(self, master):
        self.master = master
        self.lineList = []
        self.xPos = 0

        self.maxLine = 122

        self.responseIndex = 0 


        self.responseWidth = 100
        self.responseTimeCanvas = Canvas(self.master, height=self.responseWidth)
        self.responseTimeCanvas.pack()

        self.UpdateResponseTimeGraph()

    def UpdateResponseTimeGraph(self):
        self.lineLength   = random.randint(10,99)

        if len(self.lineList) >= self.maxLine:
            self.lineLength = random.randint(5,95)
            self.responseTimeCanvas.coords(self.lineList[self.responseIndex % self.maxLine], self.xPos, self.responseWidth, self.xPos + 4, self.responseWidth-self.lineLength)

            self.responseTimeCanvas.itemconfig(self.lineList[self.responseIndex % self.maxLine], fill=RGB(100, 255, 100))
        else:
            self.lineList.append(self.responseTimeCanvas.create_rectangle(self.xPos, self.responseWidth, self.xPos + 4, self.responseWidth-self.lineLength, 
                                                fill=RGB(100, 255, 100), outline=''))


        self.xPos += 5 #will cause the next line to start 5 pixels later. 
        self.responseIndex += 1

        self.responseTimeCanvas.config(scrollregion=self.responseTimeCanvas.bbox(ALL))

        self.responseTimeCanvas.xview_moveto(1.0) #move to the end of the canvas which is scrollable.



        self.responseTimeCanvas.after(10, self.UpdateResponseTimeGraph)


mw = Tk()
mainUI = MainUI(mw)
mw.mainloop()
Dave1551
  • 323
  • 2
  • 13