Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How correctly free memory from SignalListItemFactory in teardown_cb #145

Open
gavr123456789 opened this issue Jun 29, 2021 · 8 comments
Open

Comments

@gavr123456789
Copy link

C: https://github.com/ToshioCP/Gtk4-tutorial/blob/main/src/misc/list1.c#L27
Rust: https://github.com/JMS55/whattheframe/blob/b3852764eb886147c1be478b577143ba897c7592/whattheframe/src/frame_view/task_tree.rs#L49

Nim todo example:

import gintro/[gtk4, gobject, gio]
import std/with

type
  AddControlData = tuple 
    entry: Entry
    list: StringList
  DeleteControlData = tuple
    selection: SingleSelection
    list: StringList

### Utils
func getNItems(self: StringList): int = 
  let lm = cast[ListModel](self)
  return lm.getNItems()

proc getString(self: ListItem): string = 
  let strobj = cast[StringObject](self.getItem())
  result = gtk4.getString(strobj)

func generate100Strings(itemNum: int): seq[string] = 
  for index in itemNum..itemNum + 100:
    result.add "item №" & $index

### SignalFactory callbacks
proc setup_cb(factory: gtk4.SignalListItemFactory, listitem: gtk4.ListItem) =
  listitem.setChild(newButton(""))
  
proc bind_cb(factory: gtk4.SignalListItemFactory, listitem: gtk4.ListItem) =
  listitem.getChild().Button.label = listitem.getString()


proc unbind_cb(factory: gtk4.SignalListItemFactory, listitem: gtk4.ListItem) =
  # echo "unbind"
  discard

proc teardown_cb(factory: gtk4.SignalListItemFactory, listitem: gtk4.ListItem) =
  # echo "teardown_cb"
  if listitem.getChild != nil:
    echo "ref count", listitem.child.refCount
  else:
    echo "listitem.getChild = nil"

  listitem.setChild (nil)


### Controls callbacks
func btnAddCb(btn: Button, controlData: AddControlData) =
  if controlData.entry.text != "":
    controlData.list.append controlData.entry.text
    controlData.entry.text = ""
  
func btnRemoveCb(btn: Button, data: DeleteControlData) =
  data.list.remove data.selection.getSelected()
  
func btnRemoveAllCb(btn: Button, data: StringList) =
  data.splice(0, data.getNItems())

func btnAdd100Cb(btn: Button, data: StringList) =
  let maxItemPosition = data.getNItems()
  data.splice(maxItemPosition, 0, generate100Strings(maxItemPosition))

proc activate(app: gtk4.Application) =
  let
    # main
    window = newApplicationWindow(app)
    scrolled = newScrolledWindow()
    mainBox = newBox(Orientation.vertical, 0)
    # ListView
    sl = gtk4.newStringList("Nim", "Vala", "Rust", "Zig")
    ls = cast[ListModel](sl)
    ns = gtk4.newSingleSelection(ls)
    factory = gtk4.newSignalListItemFactory()
    lv = newListView(ns, factory)
    # Controls
    controlBox = newBox(Orientation.horizontal, 0)
    add = newButton("Add")
    remove = newButton("Remove")
    removeAll = newButton("Remove All")
    add100 = newButton("Add 100")
    entry = newEntry()

  # Connect controls
  add.connect("clicked", btnAddCb, (entry, sl))
  remove.connect("clicked", btnRemoveCb, (ns, sl))
  removeAll.connect("clicked", btnRemoveAllCb, sl)
  add100.connect("clicked", btnAdd100Cb, sl)

  
  with controlBox:
    halign= Align.center
    setCssClasses("linked")
    append add
    append remove
    append removeAll
    append add100
    append entry
  
  with scrolled:
    setChild lv
    vexpand = true

  with factory:
    connect("setup", setup_cb)
    connect("bind", bind_cb)
    connect("unbind", unbind_cb)
    connect("teardown", teardown_cb)

  with mainBox:
    append scrolled
    append controlBox

  with window:
    defaultSize = (500, 300)
    title = "Nim Simple Todo Example"
    setChild mainBox
    show

proc main =
  let app = newApplication("org.gtk.example")
  app.connect("activate", activate)
  discard run(app)

main()

As you can see, if add 100 -> 1000 and then remove all, every listitem.child is nil already, but memory leaks.
App on start:
image
After some tasks add and clean ups:
image

Have this problem in real app:

Peek.2021-06-30.02-45.mp4
@StefanSalewski
Copy link
Owner

Thanks for reporting.

Indeed listitem.setChild(nil) is the correct way.

But I have to admit that I have never really debugged memory leaks in GTK. It is difficult, as GTK leaks itself in some way. When we compile and run GTK C examples with valgrind we get reported leaks and other issues. I asked on the GTK forum some years ago about that, the suggested solution was difficult, so I never tried. But I generally check that destructors are called. Will investigate it soon.

@StefanSalewski
Copy link
Owner

Are you sure your observed leak is a bindings issue?

I can not discover Nim code that may generate it. And using valgrind reports many OpenGL related allocations. So we would have to first convert your Nim code to plain C and see if that is really leak free. Your linked c code example seems to be not identical in behaviour to your Nim example.

@gavr123456789
Copy link
Author

Not sure, need ask on gtk discourse.

@StefanSalewski
Copy link
Owner

Actually I can not confirm a serious memory leak.

Compile your code above with

nim c --gc:arc -d:useMalloc t.nim

$ nim -v
Nim Compiler Version 1.5.1 [Linux: amd64]
Compiled at 2021-06-21
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: ad70a65e0e3eda39a3ca074af9feadb68f10598f
active boot switches: -d:release

Run executable with

/usr/bin/time -v ./t

When we immediately terminate the program we get:

Maximum resident set size (kbytes): 61996

When we play a bit with it including clicking "add100" we get

Maximum resident set size (kbytes): 68396

When we play more with it, using "add100" multiple times but finally clear the list we get

Maximum resident set size (kbytes): 67628

So no increase.

Of course 68 MB peak size for such a tiny program is a lot. And valgrind reports leaks, but valgrind does that for all GTK programs for me, for plain C ones too. GTK is internally not really free of tiny leaks. I asked once Mr Bassi, they call it intern allocations which can not be freed. To investigate that in detail one would need valgrind white lists, I was told...

If you still think that we have a serious leak give us detailed instructions how you compiled your program, with which compiler version and which C backend, and how you measured the leak.

Note that we used -d:useMalloc for our tests, as without it the Nim memory system may reserve buffers and do not return all to the OS.

@gavr123456789
Copy link
Author

gavr123456789 commented Jul 10, 2021

Actually I can not confirm a serious memory leak

Yea, leaks in this tiny example are very small.
In real app as ListItemChild Im using object that was inherited from Box.
https://github.com/gavr123456789/Katana/blob/master/list_view.nim#L19
https://github.com/gavr123456789/Katana/blob/master/row_widget.nim#L6
May be thats the problem, I think I need to remake more complex example with DirectoryList on Vala anyway.

@gavr123456789
Copy link
Author

Example with inherited items

import gintro/[gtk4, gobject, gio]
import std/with

type
  AddControlData = tuple 
    entry: Entry
    list: StringList
  DeleteControlData = tuple
    selection: SingleSelection
    list: StringList
  MyBox = ref object of Box
    btn1: Button
    btn2: Button
    data: string 


### Utils
func getNItems(self: StringList): int = 
  let lm = cast[ListModel](self)
  return lm.getNItems()

proc getString(self: ListItem): string = 
  let strobj = cast[StringObject](self.getItem())
  result = gtk4.getString(strobj)

func generate100Strings(itemNum: int): seq[string] = 
  for index in itemNum..itemNum + 100:
    result.add "item №" & $index



### SignalFactory callbacks
proc setup_cb(factory: gtk4.SignalListItemFactory, listitem: gtk4.ListItem) =
  let x = newBox(MyBox, Orientation.horizontal, 0)
  x.setCssClasses("linked")
  x.btn1 = newButton("")
  x.btn2 = newButton("")
  x.append x.btn1
  x.append x.btn2
  x.data = "qweqweqweqweqweqweqweqweqweqweqweqweqweqweqweqweqweqweqweqweqwe"
  listitem.setChild(x)
  
proc bind_cb(factory: gtk4.SignalListItemFactory, listitem: gtk4.ListItem) =
  let 
    data = listitem.getString()
    myBox = listitem.getChild().MyBox
  myBox.btn1.label = data
  myBox.btn2.label = data


proc unbind_cb(factory: gtk4.SignalListItemFactory, listitem: gtk4.ListItem) =
  discard

proc teardown_cb(factory: gtk4.SignalListItemFactory, listitem: gtk4.ListItem) =
  if listitem.getChild != nil:
    echo "ref count", listitem.child.refCount
  else:
    echo "listitem.getChild = nil"

  listitem.setChild (nil)



### Controls callbacks
func btnAddCb(btn: Button, controlData: AddControlData) =
  if controlData.entry.text != "":
    controlData.list.append controlData.entry.text
    controlData.entry.text = ""
  
func btnRemoveCb(btn: Button, data: DeleteControlData) =
  data.list.remove data.selection.getSelected()
  
func btnRemoveAllCb(btn: Button, data: StringList) =
  data.splice(0, data.getNItems())

func btnAdd100Cb(btn: Button, data: StringList) =
  let maxItemPosition = data.getNItems()
  data.splice(maxItemPosition, 0, generate100Strings(maxItemPosition))

proc activate(app: gtk4.Application) =
  let
    # main
    window = newApplicationWindow(app)
    scrolled = newScrolledWindow()
    mainBox = newBox(Orientation.vertical, 0)
    # ListView
    sl = gtk4.newStringList("Nim", "Vala", "Rust", "Zig")
    ls = cast[ListModel](sl)
    ns = gtk4.newSingleSelection(ls)
    factory = gtk4.newSignalListItemFactory()
    lv = newListView(ns, factory)
    # Controls
    controlBox = newBox(Orientation.horizontal, 0)
    add = newButton("Add")
    remove = newButton("Remove")
    removeAll = newButton("Remove All")
    add100 = newButton("Add 100")
    entry = newEntry()

  # Connect controls
  add.connect("clicked", btnAddCb, (entry, sl))
  remove.connect("clicked", btnRemoveCb, (ns, sl))
  removeAll.connect("clicked", btnRemoveAllCb, sl)
  add100.connect("clicked", btnAdd100Cb, sl)

  
  with controlBox:
    halign= Align.center
    setCssClasses("linked")
    append add
    append remove
    append removeAll
    append add100
    append entry
  
  with scrolled:
    setChild lv
    vexpand = true

  with factory:
    connect("setup", setup_cb)
    connect("bind", bind_cb)
    connect("unbind", unbind_cb)
    connect("teardown", teardown_cb)

  with mainBox:
    append scrolled
    append controlBox

  with window:
    defaultSize = (500, 300)
    title = "Nim Simple Todo Example"
    setChild mainBox
    show

proc main =
  let app = newApplication("org.gtk.example")
  app.connect("activate", activate)
  discard run(app)

main()

@gavr123456789
Copy link
Author

gavr123456789 commented Jul 10, 2021

ListView always render only 200 items, then it start to reuse items that already was rendered (just set new content for them)
The amount of occupied memory does not increase until it is cleared. Then, after cleaning, it grows again by 2-4 megabytes and stops again, you can add at least 2000 elements. But again, as soon as I clear the list, the new 200 items will add memory, and the old one will not be released.

Peek.2021-07-10.18-48.mp4

Maybe it is the data added to the Box that flows here using inheritance (btn1, btn2 and data)

Also if:

if listitem.getChild != nil:
    echo "ref count", listitem.child.refCount

doesnt work, and it prints listitem.getChild = nil in console on every delete, so looks like child is null already.
May be its because how I clear the list:

func btnRemoveAllCb(btn: Button, data: StringList) =
  data.splice(0, data.getNItems())

@StefanSalewski
Copy link
Owner

OK, I think then we have to create a plain C example an test that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants