Bug 205070 - [JFace] ImageDataImageDescriptor's hashCode doesn't correspond to its equals method
Summary: [JFace] ImageDataImageDescriptor's hashCode doesn't correspond to its equals ...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.3.2   Edit
Assignee: Tod Creasey CLA
QA Contact:
URL:
Whiteboard: hasPatch
Keywords: contributed, performance
Depends on:
Blocks: 175526 207075
  Show dependency tree
 
Reported: 2007-10-01 11:26 EDT by Nick Edgar CLA
Modified: 2008-01-24 09:10 EST (History)
5 users (show)

See Also:

Attachments
Patch (1.23 KB, patch)
2007-10-23 13:53 EDT, Tod Creasey CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Edgar CLA 2007-10-01 11:26:35 EDT 
3.3

ImageDataImageDescriptor.hashCode() and equals don't correspond when it's created from an original image.

Note that a new ImageData object is created each time, so comparing hashCode/equals on the resulting ImageData does not allow supposedly-equivalent image descriptors to compare (or hash) properly.

This can result in leaks when the image descriptor is used in a resource manager.

Suggested change:
    public int hashCode() {
        if (originalImage != null) {
            return System.identityHashCode(originalImage);
        }
        return data.hashCode();
    }
Comment 1 Nick Edgar CLA 2007-10-01 11:52:48 EDT 
Note that ImageDataImageDescriptor's use of identity hash and ==, rather than Image.hashCode() and Image.equals(), is deliberate.  We might want to mention this in the code.  The reason for using these is that Image.hashCode() changes when the image is disposed, which also causes leaks when using resource managers.
Comment 2 Nick Edgar CLA 2007-10-01 12:31:25 EDT 
Back link to our bug db: #32291.
Comment 3 Nick Edgar CLA 2007-10-01 13:04:57 EDT 
This would be a good candidate for 3.3.2.
Comment 4 Michael Valenta CLA 2007-10-01 13:18:18 EDT 
On a related note, earlier in 3.4, I did modify DecorationOverlayIcon to use
the identity hashcode for the base image for the same reason (see bug 175526). It may be worthwhile considering backporting that change as well.
Comment 5 Tod Creasey CLA 2007-10-04 08:23:10 EDT 
This is certainly a good candidate. McQ?
Comment 6 Tod Creasey CLA 2007-10-19 12:59:23 EDT 
I have released this patch to the 3.4 stream for build >20071026. McQ I think this should go into 3.3.2
Comment 7 Tod Creasey CLA 2007-10-19 13:35:25 EDT 
Patch released for 3.3.2 build >20071019
Comment 8 Tod Creasey CLA 2007-10-22 16:19:59 EDT 
This change is causing the ResourceManagerTest suite to fail.
Comment 9 Tod Creasey CLA 2007-10-22 16:26:21 EDT 
Checking the code for the test it turns out that the test was treating some entries as different that were in fact dups.

Stefan and Nick if you could check ResourceManagerTest for me to make sure I am reading it right but I think the number of duplicates is 11 not 7 as the result for Bug 207075 indicates.
Comment 10 Mike Wilson CLA 2007-10-23 11:04:37 EDT 
Please attach a patch to the bug.
Comment 11 Tod Creasey CLA 2007-10-23 13:53:13 EDT 
Created attachment 80973 [details]
Patch
Comment 12 Mike Wilson CLA 2007-10-24 15:00:48 EDT 
+1 for adding this to 3.3.2.
Comment 13 Tod Creasey CLA 2007-10-29 11:11:30 EDT 
Verified in 3.4 M3 by the results of the I builds.
Comment 14 Tod Creasey CLA 2008-01-24 09:10:25 EST 
This test has been in the 3.3.2 builds since November 28, 2007 so it is verified in 3.3.2