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

Document ?self parameter disownment #408

Closed
bradrn opened this issue May 26, 2023 · 5 comments
Closed

Document ?self parameter disownment #408

bradrn opened this issue May 26, 2023 · 5 comments

Comments

@bradrn
Copy link

bradrn commented May 26, 2023

While implementing Kleidukos/ghcup-gtk#7, I discovered that the ?self implicit parameter is disowned as soon as the event callback finishes (since it’s implemented using withTransient). As far as I can tell, this is not documented anywhere, even though accidentally using ?self later can easily result in nasty bugs. It would be nice to document this behaviour, though I’m not sure where the best place to do so would be.

@garetxe
Copy link
Collaborator

garetxe commented May 28, 2023

Yes, indeed! This should be documented much better, it's a subtle point. The ?self implicit parameter does serve a role, but it can be dangerous. And unfortunately I could not think of a way of exposing what it does safely.

Just so I have a bit more background, what were you attempting to do, and which bugs did you encounter? I will certainly add documentation, and knowing this will help me determine the best thing to write.

@bradrn
Copy link
Author

bradrn commented May 28, 2023

Just so I have a bit more background, what were you attempting to do, and which bugs did you encounter?

I wasn’t doing anything very fancy at all… you can find the code in Kleidukos/ghcup-gtk#7. My problem was simply that I (and others) thought that the ?self parameter could be used in all the same ways as the original object, and then got a big surprise when objects started being disowned.

(My specific problem was that I needed to set up one event handler from within another, and didn’t realise that using the outer handler’s ?self from within the inner handler was dangerous. But I can imagine other ways to trigger this problem.)

garetxe pushed a commit that referenced this issue May 29, 2023
So far we were marking it as transient, but this is not necessary, and
leads to subtle problems, see

#408
garetxe pushed a commit that referenced this issue May 29, 2023
So far we were marking it as transient, but this is not necessary, and
leads to subtle problems, see

#408
@garetxe
Copy link
Collaborator

garetxe commented May 29, 2023

Could you please try haskell-gi-0.26.5 in hackage? It should fix this issue, and allow you to use ?self as any other object inside signal handlers, including passing it to other even handlers you connect inside the signal. At least in my testing it works OK. If it doesn't work please do let me know.

What I did is get rid of the withTransient in the signal handler, which is needed for boxed arguments, but not ?self, I think, and instead wrap the ?self pointer into an ordinary (refcounted) ForeignPtr. (When I wrote the code I was concerned about widget::destroy callbacks, but it seems like it's OK to add a ref in them and release it later.)

@bradrn
Copy link
Author

bradrn commented May 31, 2023

Just checked, and it does indeed seem to be fixed now, thanks!

@garetxe
Copy link
Collaborator

garetxe commented May 31, 2023

Great, thanks for checking!

bradrn added a commit to bradrn/ghcup-gtk that referenced this issue May 31, 2023
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