[ https://issues.jboss.org/browse/JBIDE-10247?page=com.atlassian.jira.plugi... ] Viacheslav Kabanovich commented on JBIDE-10247: ----------------------------------------------- I cannot see the problem with this class. Take handling 'rightButton' as an example: {code} Image rightImage = rightDesc.createImage(); rightButton.setImage(rightImage); rightButton.setEnabled(false); rightButton.addDisposeListener(new DisposeListener() { public void widgetDisposed(DisposeEvent e) { Button button = (Button) e.getSource(); button.getImage().dispose(); } }); {code} The dispose listener takes button.getImage() and it returns rightImage because that image is never replaced. Vitali, what may cause leak here? -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/JBIDE-10247?page=com.atlassian.jira.plugi... ] Viacheslav Kabanovich commented on JBIDE-10247: ----------------------------------------------- Class Button does not override methods Widget.dispose() and Control.setEnabled(). Comments to method Control.setEnabled() say nothing to the effect that client must explicitly invoke dispose() on an object, on which setEnabled() was invoked. Comments to method Widget.dispose() say that it "Disposes of the operating system resources associated with the receiver and _all its descendants_". When dialog is closed, the tree of its controls is disposed and that is why we receive the dispose event. Button overrides package local methods releaseHandle() and releaseWidget(), which are invoked for each widget in the tree, and they should release all internally created resources. If a leak tracking tool detects a leak in resources internally created by Button, it is Eclipse's bug and should be reported to Eclipse. I shudder at the idea of invoking dispose() explicitly on each widget. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/JBIDE-10247?page=com.atlassian.jira.plugi... ] Viacheslav Kabanovich commented on JBIDE-10247: ----------------------------------------------- Method dispose() is not called recursively, that is true. Here is its implementation: {code} public void dispose () { /* * Note: It is valid to attempt to dispose a widget * more than once. If this happens, fail silently. */ if (isDisposed ()) return; if (!isValidThread ()) error (SWT.ERROR_THREAD_INVALID_ACCESS); release (true); } {code} It invokes method release(true). Method release(boolean) is called recursively. That means, that if dispose() is invoked on a root composite of the dialog, release(true) will be invoked for each widget in the tree, and DisposeEvent will be sent to each widget. Also, please note that Widget.release(boolean) is package local and not documented, it is not a part of API and may be subjected to changes without notice. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/JBIDE-10247?page=com.atlassian.jira.plugi... ] Vitali Yemialyanchyk commented on JBIDE-10247: ---------------------------------------------- i.e. I can not state, what this is JBTools issue, this could be SWT issue, but easiest way to fix it -> just call dispose for button in widgetDisposed listener. may be this is SWT bug, or may be SWT developers can state, what this is expected behavior - from documentation it look like standard behavior, imo. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/JBIDE-10247?page=com.atlassian.jira.plugi... ] Viacheslav Kabanovich commented on JBIDE-10247: ----------------------------------------------- Widget.release {code} void release (boolean destroy) { ... releaseChildren (destroy); ... } {code} Composite.releaseChildren {code} void releaseChildren (boolean destroy) { Control [] children = _getChildren (); for (int i=0; i<children.length; i++) { Control child = children [i]; if (child != null && !child.isDisposed ()) { child.release (false); } } super.releaseChildren (destroy); } {code} Vitali, comment to dispose() says two important things 1) It promises to "Dispose of the operating system resources associated with the receiver and all its descendants". 2) dispose() is not called recursively. Please do not look for a contradiction here. In my opinion, the behavior (2) is implemented to discourage overriding dispose() method, because custom implementation would have to call super.dispose(), and a lot of programmers would forget it and cause a lot of trouble. By means of (2), Eclipse can implement (1) consistently and reliably. I repeat that if your tool detected leak of resource in Button, it must be SWT bug, please report it to Eclipse, it is critical, generally. In particular case of CSSSelectorPartComposite it is not critical, but we have a lot of places where button.setEnabled() is invoked. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira