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

Refactor SetFinalizer and toggle notifier #93

Open
diamondburned opened this issue Feb 15, 2023 · 2 comments
Open

Refactor SetFinalizer and toggle notifier #93

diamondburned opened this issue Feb 15, 2023 · 2 comments

Comments

@diamondburned
Copy link
Owner

Preamble

Ideally, closures should not hold any kind of reference to GObjects. C code doesn't do this: there's no capturing, and closures all take and release references as they're called instead of holding a reference for as long as the object is alive.

Currently, Go's finalizer fails us. Ou closures reference objects throughout the duration of the program, causing a finalizer cycle. This Can we make the finalizer do less and do our work in the toggle notifier?

Proposal

  • Go should hold a strong or weak reference to the object; the finalizer unrefs it
  • We'll handle freeing the object using a C-based toggle notifier

Can we get away with using WeakRefs for the Go object? Is there a way to combine both that and the Go finalizer to detect when to actually free?

@diamondburned
Copy link
Owner Author

Alternative

An interesting alternative would be to make objects floating/weak by default. Users who want to revive a GObject later on must manually sink the reference with .Sink() or be extra careful with how they take the object away from C memory.

If we do this, how would subclassed objects work? We would have to keep a Go reference. This might overcomplicate things.

@diamondburned
Copy link
Owner Author

diamondburned commented Mar 3, 2023

Alternative

Have a WeakBox structure:

type SignalBox coreglib.SignalBox[Main]
signalBox := SignalBox{main}

s.ConnectOpen(func(signalBox *SignalBox) {
  // use signalBox
}, signalBox)

This is guaranteed to work, but it sucks.

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

1 participant