Skip to content

Fix finalizers implementation for Opaque subclasses#55

Merged
thiblahute merged 2 commits into
GLibSharp:masterfrom
ylatuya:fix-opaque-finalizers
Jul 29, 2021
Merged

Fix finalizers implementation for Opaque subclasses#55
thiblahute merged 2 commits into
GLibSharp:masterfrom
ylatuya:fix-opaque-finalizers

Conversation

@ylatuya

@ylatuya ylatuya commented Jul 2, 2021

Copy link
Copy Markdown
Collaborator

There was one finalizer being for level of inheritance.
For example Gst.Message -> Gst.MiniObject -> GLib.Opaque ends
with 2 finalizers being called, one calling gst_mini_object_unreg
another one calling gst_message_unref
The following implementation will only call once the unref function
for the highest class in the hierarchy

Fixes #54

@ylatuya ylatuya force-pushed the fix-opaque-finalizers branch from 3c4fe0c to 05839f6 Compare July 5, 2021 13:29
@ylatuya

ylatuya commented Jul 6, 2021

Copy link
Copy Markdown
Collaborator Author

I don't have a the correct set-up to re-generate all the sources. I would need someone with the latest sources of all deps to correctly regenerate the bindings.

ylatuya added 2 commits July 6, 2021 17:02
There was one finalizer being called for each level of inheritance.
For example Gst.Message -> Gst.MiniObject -> GLib.Opaque ends
with 2 finalizers being called, one calling gst_mini_object_unref
and another one calling gst_message_unref
The following implementation will only call once the unref function
for the highest class in the hierarchy

Fixes GLibSharp#54
@ylatuya ylatuya force-pushed the fix-opaque-finalizers branch 2 times, most recently from 38ef8ba to 61b0b33 Compare July 6, 2021 15:13
@ylatuya ylatuya changed the title WIP: Fix finalizers implementation for Opaque subclasses Fix finalizers implementation for Opaque subclasses Jul 6, 2021
@ylatuya

ylatuya commented Jul 6, 2021

Copy link
Copy Markdown
Collaborator Author

Everything is ready from my side now and the PR is ready for review

@thiblahute

Copy link
Copy Markdown
Contributor

Have you written a patch for gst-sharp already?

@ylatuya

ylatuya commented Jul 7, 2021

Copy link
Copy Markdown
Collaborator Author

I am still setting up the dev env from scratch in macOS, once done I will create a patch for gstreamer-sharp and run the unit tests I have for module using it, which is what triggered the issues with the finalizers.

@thiblahute

Copy link
Copy Markdown
Contributor

Should we wait for the GSTreamer patches to be ready/tested first?

@ylatuya

ylatuya commented Jul 8, 2021

Copy link
Copy Markdown
Collaborator Author

I have been able to test the patch with GStreamer 1.18.0 and everything works correctly now without glib warnings nor crashes.
I am unable to build GStreamer from master with GI due to some build issues that I am unable to quickly fix, so I won't be able to provide a patch for gstreamer-sharp from the master branch.

@ylatuya

ylatuya commented Jul 8, 2021

Copy link
Copy Markdown
Collaborator Author

I created the following MR in gstreamer-sharp for 1.18 https://gitlab.freedesktop.org/gstreamer/gstreamer-sharp/-/merge_requests/30

Comment thread Source/generator/OpaqueGen.cs
@thiblahute thiblahute merged commit 6a286d3 into GLibSharp:master Jul 29, 2021
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

Successfully merging this pull request may close these issues.

Objects inheriting from objects that inherit from GLib.Opaque are unrefing their handle twice.

3 participants