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

Call g_object_ref_sink() only on floating objects #80

Open
StefanSalewski opened this issue Jul 2, 2020 · 5 comments
Open

Call g_object_ref_sink() only on floating objects #80

StefanSalewski opened this issue Jul 2, 2020 · 5 comments

Comments

@StefanSalewski
Copy link
Owner

There is a bug in template gobjectTemp(): untyped for gintro <= v0.7.7

methodBuffer.writeLine(" discard g_object_ref_sink(result.impl)")

ref_sink should be called only for floating objects like most widgets, but not for GtkWindow, GtkApplicationWindow and other none floating windows.

@sdroege
Copy link

sdroege commented Jan 17, 2021

It seems like you currently don't handle floating references at all: there is no call to g_object_ref_sink() anywhere. This most likely means that you're leaking memory or keep dangling pointers around.

It is necessary for bindings to handle floating references correctly (and not expose them to the target language!). See https://developer.gnome.org/gobject/stable/gobject-The-Base-Object-Type.html#floating-ref for details how this is supposed to work, but for one thing you'll have to call g_object_ref_sink() on every transfer none return value of types with floating references. You can call g_object_ref_sink() also on other types and it behaves exactly the same as g_object_ref() in that case, if that makes anything easier for you.

See also #108 (comment) which suggests you handle transfer none wrong in general.

@StefanSalewski
Copy link
Owner Author

It seems like you currently don't handle floating references at all:

salewski@nuc ~ $ grep g_object_ref_sink ~/.nimble/pkgs/gintro-#head/gintro/* |wc -l
5345

It is nice that you try to help. But most of gintro seems to work fine. I would have to test for memory leaks more indeed.

@sdroege
Copy link

sdroege commented Jan 17, 2021

Hmm searching in this repository for ref_sink gave no hits at all so I might be missing something.

@StefanSalewski
Copy link
Owner Author

The bindings modules are created by the gen.nim generator script during install of the gintro module, that is

nimble install gintro

or

nimble install gintro@head

to get latest changes (recommended currently). That will give you

salewski@nuc ~/GtkProgrammingBook $ ls -lt ~/.nimble/pkgs/gintro-#head/gintro
total 11908
-rw-r--r-- 1 salewski salewski 2308858 Jan 16 16:48 gtk4.nim
-rw-r--r-- 1 salewski salewski  296367 Jan 14 15:59 gtksource5.nim
-rw-r--r-- 1 salewski salewski  268709 Jan 14 15:59 gtksource.nim
-rw-r--r-- 1 salewski salewski  226242 Jan 14 15:59 handy.nim
-rw-r--r-- 1 salewski salewski  177614 Jan 14 15:59 harfbuzz.nim
-rw-r--r-- 1 salewski salewski   86213 Jan 14 15:59 javascriptcore.nim
-rw-r--r-- 1 salewski salewski   47202 Jan 14 15:59 nice.nim
-rw-r--r-- 1 salewski salewski   12904 Jan 14 15:59 notify.nim
-rw-r--r-- 1 salewski salewski   11606 Jan 14 15:59 pangocairo.nim
-rw-r--r-- 1 salewski salewski    8156 Jan 14 15:59 pangoft2.nim
-rw-r--r-- 1 salewski salewski  176448 Jan 14 15:59 pango.nim
-rw-r--r-- 1 salewski salewski   31282 Jan 14 15:59 rsvg.nim
-rw-r--r-- 1 salewski salewski  297579 Jan 14 15:59 soup.nim
-rw-r--r-- 1 salewski salewski   78947 Jan 14 15:59 vte.nim
-rw-r--r-- 1 salewski salewski  427177 Jan 14 15:59 webkit2.nim
-rw-r--r-- 1 salewski salewski  820834 Jan 14 15:59 webkit2webextension.nim
-rw-r--r-- 1 salewski salewski    1445 Jan 14 15:59 xlib.nim
-rw-r--r-- 1 salewski salewski   81923 Jan 14 15:59 gisup3.nim
-rw-r--r-- 1 salewski salewski   52645 Jan 14 15:59 gisup4.nim
-rw-r--r-- 1 salewski salewski  429183 Jan 14 15:59 glib.nim
-rw-r--r-- 1 salewski salewski    1886 Jan 14 15:59 gmodule.nim
-rw-r--r-- 1 salewski salewski   90421 Jan 14 15:59 gobject.nim
-rw-r--r-- 1 salewski salewski   81840 Jan 14 15:59 graphene.nim
-rw-r--r-- 1 salewski salewski  116921 Jan 14 15:59 gsk.nim
-rw-r--r-- 1 salewski salewski   16750 Jan 14 15:59 gstapp.nim
-rw-r--r-- 1 salewski salewski  118863 Jan 14 15:59 gstbase.nim
-rw-r--r-- 1 salewski salewski  666232 Jan 14 15:59 gst.nim
-rw-r--r-- 1 salewski salewski 2708779 Jan 14 15:59 gtk.nim
-rw-r--r-- 1 salewski salewski  107752 Jan 14 15:59 cairoimpl.nim
-rw-r--r-- 1 salewski salewski   11187 Jan 14 15:59 cairo.nim
-rw-r--r-- 1 salewski salewski      74 Jan 14 15:59 dummygtk.nim
-rw-r--r-- 1 salewski salewski     446 Jan 14 15:59 fontconfig.nim
-rw-r--r-- 1 salewski salewski     569 Jan 14 15:59 freetype2.nim
-rw-r--r-- 1 salewski salewski  347723 Jan 14 15:59 gdk4.nim
-rw-r--r-- 1 salewski salewski  377339 Jan 14 15:59 gdk.nim
-rw-r--r-- 1 salewski salewski  107262 Jan 14 15:59 gdkpixbuf.nim
-rw-r--r-- 1 salewski salewski   30847 Jan 14 15:59 gdkx114.nim
-rw-r--r-- 1 salewski salewski   31953 Jan 14 15:59 gdkx11.nim
-rw-r--r-- 1 salewski salewski    2564 Jan 14 15:59 gimplglib.nim
-rw-r--r-- 1 salewski salewski   12457 Jan 14 15:59 gimplgobj.nim
-rw-r--r-- 1 salewski salewski    7631 Jan 14 15:59 gimplgtk.nim
-rw-r--r-- 1 salewski salewski    3953 Jan 14 15:59 gimplnice.nim
-rw-r--r-- 1 salewski salewski 1270554 Jan 14 15:59 gio.nim
-rw-r--r-- 1 salewski salewski  131491 Jan 14 15:59 atk.nim
-rw-r--r-- 1 salewski salewski    2718 Jan 13 16:57 gimplgst.nim

Indeed Mr. Rumpf does not like these "on the fly" generation that much, I think we already discussed it too. Later, when we may have a few thousand or at least a few hundred Nim GTK users we may try to ship pre-generated module files. But currently I can not provide these, as that would mean creating weekly fresh files for Linux, Mac and Windows, maybe for 32 and 64 bit.

@StefanSalewski
Copy link
Owner Author

Sorry, it is

nimble install gintro@#head

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