Memory leak in `connect()` template

Following code expands connect() template using expandMacros.

import gintro/[dummygtk, gio, gobject, glib]
import macros

var gMainLoop = newMainLoop(nil, false)

proc cbChanged(self: FileMonitor; file: GFile; otherFile: GFile = nil; eventType: FileMonitorEvent, x: int) =
  echo "File changed", x
  gMainLoop.quit

proc main =
  "foo.txt".writeFile("")
  var
    f = newGFileForPath("foo.txt")
    fmon = f.monitorFile({})

  var x = 123
  expandMacros:
    fmon.connect("changed", cbChanged, x)
  "foo.txt".writeFile("a")
  gMainLoop.run

main()

and this is expanded code:

proc connect_for_signal_cdecl_changed1(self: ptr FileMonitor00;
                                       file: ptr GFile00;
                                       otherFile: ptr GFile00;
                                       eventType: FileMonitorEvent;
                                       user_data: pointer) {.cdecl.} =
  let h: pointer = g_object_get_qdata(self, Quark)
  var file1: GFile
  if isNil(file1):
    new file1
    GC_ref(file1)
    file1.ignoreFinalizer = true
  file1.impl = file
  var otherFile1: GFile
  if isNil(otherFile1):
    new otherFile1
    GC_ref(otherFile1)
    otherFile1.ignoreFinalizer = true
  otherFile1.impl = otherFile
  cbChanged(cast[gio.FileMonitor](h), file1, otherFile1, eventType,
            cast[ptr int](user_data)[])

proc connect_for_signal_changed1(self: gio.FileMonitor; p: proc (
    self: gio.FileMonitor; file: GFile; otherFile: GFile = nil;
    eventType: FileMonitorEvent; arg: int); a: int): culong {.discardable.} =
  result =
    var ar: ref int
    new(ar)
    ar[] = a
    GC_ref(ar)
    scChanged(self, connect_for_signal_cdecl_changed1, cast[pointer](ar), {})

connect_for_signal_changed1(fmon, cbChanged, x)

This code creates ref objects and calls GC_ref to them. Unless GC_unref is called to them in cbChanged (callback procedure), there is no way to access these ref object and they are leaked.

I don't think GC_ref(file1) and GC_ref(otherFile1) is needed.

ref object that holds user_data can be freed by defining generic procedure that calls GC_unref to user_data and pass it to g_signal_connect_data () called in scChanged.

proc unrefUserData[T](data: pointer; closure: ptr Closure00) {.cdecl.} =
  GC_unref(cast[T](data))

gintro manages both GObject and ref object. And gintro puts a pointer to GObject in Nim's ref object and also puts pointer to the ref object to the GObject. So reference counters in both GObject and in Nim's ref object must be managed manually. I think gintro's code can become simpler if it wraps GObject pointer with Nim's object type and only manage GObject's reference counter. This is example code(requires --gc:arc):

const Lib = "libgobject-2.0-0.so"
{.pragma: libprag, cdecl, dynlib: Lib.}

type
  Object00*{.inheritable, pure.} = object
  Object = object of RootObj
    impl*: ptr Object00

proc g_object_ref*(self: ptr Object00): ptr Object00 {.
    discardable, importc, libprag.}

proc g_object_unref*(self: ptr Object00) {.
    importc, libprag.}

proc `=destroy`*(o: var Object) =
  if o.impl != nil:
    g_object_unref(o.impl)

proc `=copy`*(o1: var Object; o2: Object) =
  if o1.impl == o2.impl:
    return
  `=destroy`(o1)
  wasMoved(o1)
  o1.impl = o2.impl
  if o1.impl != nil:
    g_object_ref(o1.impl)

# proc `=sink`(o1: var Object; o2: Object) =
# It doesn't need to be defined as compiler is using `=destroy` and `copyMem` when not provided

proc cancelDestroy*(o: var Object) =
# Stop calling `=destroy` by setting nil to impl in case you don't want to call g_object_unref.
  o.impl = nil

# Allow setting nil to Object like someObj = nil,
# but do not allow setting any other pointer type.
# This converter need to be defined for each type inherits gobject.Object.
converter toObject*(n: typeof(nil)): Object = Object()

var a: Object
a = nil
# This is compile error.
#var n: int
#a = cast[pointer](n.addr)

#In gio.nim

{.pragma: libprag, cdecl, dynlib: "libgio-2.0-0.so".}

type
  GFile00* = object of Object00
  GFile* = object of Object

type
  FileMonitor* = object of Object
  FileMonitor00* = object of Object00
  FileMonitorEvent* {.size: sizeof(cint), pure.} = enum
    changed = 0

converter toGFile*(n: typeof(nil)): GFile = GFile()

proc g_file_new_for_path(path: cstring): ptr GFile00 {.
    importc, libprag.}

proc newGFileForPath*(path: cstring): GFile =
  GFile(impl: g_file_new_for_path(path))

# In user code
proc cbChanged(self: FileMonitor; file: GFile; otherFile: GFile = nil; eventType: FileMonitorEvent, x: int) =
  discard

# Procedure generated by connect template and it is a callback proc called when signal emitted.
proc connect_for_signal_cdecl_changed1(self: ptr FileMonitor00;
                                       file: ptr GFile00;
                                       otherFile: ptr GFile00;
                                       eventType: FileMonitorEvent;
                                       user_data: pointer) {.cdecl.} =
  var
    h = FileMonitor(impl: self)
    file1 = GFile(impl: file)
    otherFile1 = GFile(impl: otherFile)

  # `cbChanged` is a user specified callback proc.
  cbChanged(h, file1, otherFile1, eventType,
            cast[ptr int](user_data)[])

  h.cancelDestroy()
  file1.cancelDestroy()
  otherFile1.cancelDestroy()

Thanks for reporting.

I will try to investigate your ideas soon. I think I had similar thoughs in the last years. But at the same time I have some fear to make larger changes, as we have currently only a very limited set of test programs and very few actual users which could do tests. So every larger change could introduce undetected regression. And then maybe in 10 years someone detects a regression and nobody can remember details. The fact that Araq and other Nim core devs do not like GTK does not help. Araq seems to like fidget best currently, but I have not too much trust in that project and can not imagine how to create a serious GUI app like gimp or inkscape with it.

My main concern with the gintro bindings is the use of proxy objects and the toggle-ref stuff. If we would never use inheritance but only composition, them we would not need the proxy objects. At least yet with ARC and working destructors. When I started gintro five years ago we had only finalizers. With inheritance, we may use the gobject lib to extent objects maybe. Maybe I should study bindings to other languages like Rust. Asking on GTK forum is always problematic, as Mr Bassi is the only remaining GTK expert, and he has to answer 95% of all the questions already. Would be all more fun with at least a few hundred Nim GTK users :-)

I completely agree about gimp, inkscape, and fidget. GTK is a very mature and powerful framework, with full time developers and free license(qt). It is unlikely that now you can start a project that compares with 20 years of GTK development.

Currently, GTK applications are written mainly in three languages: Vala, Python, and Rust. All of these languages in my opinion have drawbacks and Nim could be the perfect language for GTK. Rust is too system-oriented and low-level for writing extensive business logic, Python is good, but dynamic typing and interpretation makes it a poor candidate for large applications. Vala was literally created for GTK and there is no more convenient experience of interacting with, but Vala itself has an insufficient number of libraries and is very slowly developed by one person in his spare time. Nim does not have any of these disadvantages, and in theory, the usability of GTK from Nim can reach the level of Vala thanks to macros.

For example, Nim would allow you to implement a simple interface formation language that is dreamed of in the GTK community (analogous to qml or apple ui kit) gnome discourse. Something like here in Rust:

view! {
        gtk::Window {
            gtk::Box {
                orientation: Vertical,
                gtk::Button {
                    // By default, an event with one paramater is assumed.
                    clicked => Msg::Increment,
                    // Hence, the previous line is equivalent to:
                    // clicked(_) => Increment,
                    label: "+",
                },
                gtk::Label {
                    // Bind the text property of this Label to the counter attribute
                    // of the model.
                    // Every time the counter attribute is updated, the text property
                    // will be updated too.
                    text: &self.model.counter.to_string(),
                },
                gtk::Button {
                    clicked => Msg::Decrement,
                    label: "-",
                },
            },
            // Use a tuple when you want to both send a message and return a value to
            // the GTK+ callback.
            delete_event(_, _) => (Msg::Quit, Inhibit(false)),
        }
    }

Or here in kotlin, C#?.

It seems to me that the fact that there are few users now, on the contrary, frees the hands for global changes.

It seems to me that the fact that there are few users now, on the contrary, frees the hands for global changes.

Yes, but the amount of work one can do in his lifespan is limited, so one has to consider if the work one is doing makes at least minimal sense.

GTK wrappers is for sure no big contribution to whole mankind as its own, like maybe a discovered mathematical theorem may be. So wrappers value is defined by number of users at the end. When I started wrapping gtk3 in 2015 there was mostly only the old low level gtk2 wrapper available for Nim, nearly no other GUI libs. So my expectation was that after a few years we would have at least a few hundred gintro users. But currently my guess is that we have between zero and maybe 5 real users. And I spent 1600 hours on the gintro wrapper, which includes the gtk3 examples. Additional 600 hours before in 2015 for the oldgtk3 low level wrapper. So one has to ask if it makes really sense. The fact that Araq does not like GTK that much makes it not really better, there is nearly no support from Nim core devs for my work, and Araq advertise more other libs like fidget. So I have not much hope that number of users will really increase.

And I have to admit that I would like a pure Nim GUI, if it would be as least so powerful as GTK and looking not worse. I would be willing to port my apps. Qt would be fine for me to, when I can do all what I can do in C++ from Nim without significant overhead. The other option would be to use a different language with very good GUI support. For Apple Swift would do the trick. Or Rust with very good GTK support.

I recently asked Mr Droege about the Rust GTK bindings: They managed indeed to get bindings without the need of proxy objects. So Rust can fully replace Vala.

So one idea of me would be to hire a core GTK and RUST devs, both maybe for 3 months, to create Nim bindings looking to the user as the current gintro ones but based on the Rust bindings. I think this would be a perfect solution. But with so few users it is very difficult to pay these devs unfortunately. Maybe Mozilla foundation could do?

Reference: https://discourse.gnome.org/t/it-is-possible-to-port-now-app-written-in-gtk3-gtk-rs-and-glade-to-gtk4/6007/5

StefanSalewski / gintro

Memory leak in `connect()` template #118