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

Auto-dispose for widgets #9387

Open
johnspackman opened this Issue Aug 31, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@johnspackman
Member

johnspackman commented Aug 31, 2017

Following on from #9375 by @iceteagroup, disposing of widgets manually causes the same problems as having to manually dispose ordinary objects - IMHO it arises less often only because many applications never remove widgets from their UI.

A clean solution will be to make wigets auto-dispose; I think that this may be possible by delaying registering widgets in qx.core.ObjectRegistry until they are actually added to the UI and de-registering them when they are removed. Because we no longer have to decouple DOM from JS, the only reason that this register/deregister is needed is for backwards compatibility, ie to allow code to be able to rely on widget.toHashCode() being listed in ObjectRegistry. The DOM objects already have been modified so that they have the widget as a property (ie not just a string of the hash, but the actual Qx object).

However, the key principal with auto-dispose is that the destructor does not do anything of any significance and the difficulty is that widgets often only do cleanup when they are destroyed - eg qx.ui.core.Widget destructor removes the widget from appearance queues, and this code would have to be moved to a new "onDetach" function.

This should be fine for backwards compatibility because any existing user code which removed objects will either call dispose() (in which case onDetach will have been called, plus the user's destructor) or they were allowing the leak so are no worse off.

There are a number of references under qx.event.handler.* where references to widgets are kept, and these could be confirmed as being cleaned up by onDetach or converted to decoupled via hash codes.

@derrell

This comment has been minimized.

Show comment
Hide comment
@derrell

derrell Aug 31, 2017

Member

👍

Member

derrell commented Aug 31, 2017

👍

@level420 level420 added the enhancement label Oct 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment