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

Manually implement FFI code for GObject instead of using glib_shared_wrapper! #547

Merged
merged 3 commits into from Dec 7, 2019

Conversation

@sdroege
Copy link
Member

sdroege commented Dec 7, 2019

This way we can implement special cases more correctly:
- g_value_get_boxed() is invalid for GObjects
- Debug impl can print the actual type
- Only from_glib_none() will ever steal floating references and e.g.
  cloning does not, see https://github.com/gtk-rs/glib/issues/545

After this the only places where we would steal floating references from
C code would be when explicitly calling from_glib_none() somewhere. That
is, when we call a function and get a (transfer none) reference to an
object back. (transfer floating) is an alias for this.

Especially, cloning and from_glib_borrow() would never steal floating
references, and because of this it is very important to only ever use
from_glib_borrow() inside callbacks, signal handlers and virtual method
trampolines.

@EPashkin @GuillaumeGomez @federicomenaquintero @MathieuDuponchelle @ebassi Does the above make sense to you?

This is basically the workaround for C code passing us floating references that we should not actually own. We try now very hard to not steal them, i.e. the only place where we would steal them now is if a function is returning a (transfer none) object to use (return value or out parameter), or more specifically if from_glib_none() is used. We won't anymore steal floating references passed into: callbacks, signal handlers, virtual methods and passed to us via GValues but keep the floating flag intact to have C worry about this mess instead.

…wrapper!

This way we can implement special cases more correctly:
- g_value_get_boxed() is invalid for GObjects
- Debug impl can print the actual type
- Only from_glib_none() will ever steal floating references and e.g.
  cloning does not, see #545

After this the only places where we would steal floating references from
C code would be when explicitly calling from_glib_none() somewhere. That
is, when we call a function and get a (transfer none) reference to an
object back. (transfer floating) is an alias for this.

Especially, cloning and from_glib_borrow() would never steal floating
references, and because of this it is very important to only ever use
from_glib_borrow() inside callbacks, signal handlers and virtual method
trampolines.
@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Dec 7, 2019

@sdroege Sound reasonable, thanks

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Dec 7, 2019

WIP: Make FloatingReferenceGuard a no-op

This commit I'd like to get rid of in a separate PR. That way the CI can stay green all the time, @GuillaumeGomez

sdroege added 2 commits Dec 7, 2019
Once all crates are updated this struct should be completely removed.
@sdroege sdroege force-pushed the sdroege:floating-references-borrows branch from d3b8425 to 2dab0e8 Dec 7, 2019
@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Dec 7, 2019

Sounds good for me as well, thanks!

@GuillaumeGomez GuillaumeGomez merged commit d999581 into gtk-rs:master Dec 7, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MathieuDuponchelle

This comment has been minimized.

Copy link
Contributor

MathieuDuponchelle commented Dec 9, 2019

Addressed the issue I had in my C test suite (uses GstHarness, which takes a simple ref and doesn't actually take ownership of the floating ref). Well done :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.