Skip to content

Conversation

fbrouille
Copy link
Contributor

Allow custom implementation of gio::FileEnumerator

.close_fn
.expect("No parent class implementation for \"close_fn\"");

// get the corresponding object instance without checking the reference count because the object might just be finalized.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

close() is called during finalize()?! That means that this can't be provided as a normal virtual method then as it's unsound to access the full object during finalization (parts might already be freed, you can easily cause memory safety issues)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to call close in dispose because in this case it is safe to access the object. See tests for an example.

I also added a comment in closet notify developers they have to implement dispose`.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't depend on developers implementing dispose for safe usage though. Calling it in dispose seems fine but you'd have to make sure this happens automatically then.

Copy link
Contributor Author

@fbrouille fbrouille Jun 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be done by adding a new method IsSubclassable::instance_dispose with default implementation which calls ObjectImpl::dispose.
Then implementation of IsSubclassable for FileEnumerator can overrides instance_dispose to call close.
Obviously extern "C" fn dispose must be modified to call IsSubclassable::instance_dispose instead of ObjectImpl::dispose
What do you think ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't depend on developers implementing dispose for safe usage though. Calling it in dispose seems fine but you'd have to make sure this happens automatically then.

We don't have a mechanism for injecting a call inside ObjectImpl's dispose. As currently for gtk4-rs composite templates, you have to call dispose_template in dispose yourself.

So I rather we don't block on this given the low usage of the API and just make sure it is clearly documented. We can revisit this once we have a clear mechanism for achieving that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me but also maybe in gio itself it should already call this in dispose instead of finalize.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can someone create an issue, also make it very clear in the docs here, and then we can get it in I guess

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a link to that to the docs here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@fbrouille fbrouille force-pushed the gio_file_enumerator_subclass branch 2 times, most recently from 9b3a128 to 07be086 Compare June 18, 2025 17:48
@fbrouille fbrouille force-pushed the gio_file_enumerator_subclass branch 2 times, most recently from 00ed370 to 8d61fbe Compare June 18, 2025 18:01
@fbrouille fbrouille force-pushed the gio_file_enumerator_subclass branch 2 times, most recently from d68a59c to 762cd61 Compare July 3, 2025 16:49
@fbrouille fbrouille force-pushed the gio_file_enumerator_subclass branch from 762cd61 to 16d5609 Compare July 4, 2025 16:48
@fbrouille fbrouille force-pushed the gio_file_enumerator_subclass branch from 16d5609 to 78c408c Compare July 4, 2025 16:56
@sdroege sdroege merged commit de81b68 into gtk-rs:main Jul 7, 2025
48 checks passed
@fbrouille fbrouille deleted the gio_file_enumerator_subclass branch July 7, 2025 11:39
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.

3 participants