-
Notifications
You must be signed in to change notification settings - Fork 20
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
Memory leak in connect()
template
#118
Comments
I don't think ref object that holds user_data can be freed by defining generic procedure that calls proc unrefUserData[T](data: pointer; closure: ptr Closure00) {.cdecl.} =
GC_unref(cast[T](data)) |
gintro manages both GObject and ref object. 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. 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)),
}
} 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. 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? |
Following code expands
connect()
template usingexpandMacros
.and this is expanded code:
This code creates ref objects and calls
GC_ref
to them.Unless
GC_unref
is called to them incbChanged
(callback procedure), there is no way to access these ref object and they are leaked.The text was updated successfully, but these errors were encountered: