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

Default KeyGenerator doesn't work for methods with arguments of array types [SPR-11505] #16130

Closed
spring-projects-issues opened this issue Mar 3, 2014 · 10 comments
Assignees
Labels
in: core status: backported type: bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Mar 3, 2014

Craig opened SPR-11505 and commented

The way that Spring calculates cache keys doesn't work for methods that take parameters of array types. For example, define this method:

@Cacheable
public String test(String[] one)
{
return "something";
}

Then run:

String[] alpha = {"a","b","c"};
String[] beta = {"a","b","c"};
test(alpha);
test(beta);

The result is 2 executions of the method body (where in my opinion, only 1 is expected) and 2 cache entries (again, only 1 is expected).

The result of this problem is that methods with array type parameters will always result in a cache miss (the body is always executed) and a cache put is always done after the method is executed (filling the cache with garbage that's never used again). This second part is particularly troublesome, as useful stuff ends up getting evicted as the cache fills with garbage.

The problem is that DefaultKeyGenerator (Spring < 4) and SimpleKeyGenerator (Spring 4 and later) use hashCode(), and the hashCode of an array isn't specifically defined, and bubbled up to Object.hashCode() which uses the memory address of the array. Since each array, even if it has the same items in it, has a different address, the hashCode is always different.

Changing the implementation to use deepEquals/deepHashCode fixes this problem.


Affects: 3.1.4, 3.2.8, 4.0.2

Referenced from: commits e50cff4, 6d8f3a0, 70155e9

Backported to: 3.2.9

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 3, 2014

Craig commented

Pull request: #477

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 4, 2014

Stéphane Nicoll commented

This has been merged, thanks.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 4, 2014

Craig commented

Here are some more pull requests:
3.1.x: #479
3.2.x: #478

Since DefaultKeyGenerator also exists in 4.x, that 3.2.x pull request should be merged up to 4.x as well.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 4, 2014

Stéphane Nicoll commented

We were already discussing the opportunity of merging this to 3.2 and this issue has been flagged as a potential candidate. If we do it, we'll update the master version too.

Could you please confirm that you have signed the CLA?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 4, 2014

Craig commented

I have signed the CLA (I think quite a while ago).

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 4, 2014

Stéphane Nicoll commented

I am flagging this as complete. We will review this one as a candidate for the backport in 3.2.9 and update the issue accordingly.

Thanks for the proposal.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 5, 2014

Stéphane Nicoll commented

Actually, this issue is still there in case of a single parameter argument that is an array.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 5, 2014

Stéphane Nicoll commented

Jürgen, I am assigning that back to you as we discussed so that you fix this alongside the backport. Thanks.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 5, 2014

Juergen Hoeller commented

Fixed the plain array parameter case for both SimpleKeyGenerator (wrapping it with a SimpleKey) and DefaultKeyGenerator (falling back to hash calculation) in master now. The latter will be backported to 3.2.9 soon.

Note that the 3.1.x line has been retired already, so we'll only fix this for 4.0.3 and 3.2.9. We strongly recommend an upgrade to 3.2.8+ in general.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 7, 2014

Juergen Hoeller commented

This is available in the latest 4.0.3 snapshot already, and will be available in the immediately upcoming 3.2.9 snapshot as well (see http://projects.spring.io/spring-framework/ for Maven coordinates). Please give it a try and let us know whether it works for you...

Juergen

@spring-projects-issues spring-projects-issues added type: bug status: backported in: core labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 4.0.3 milestone Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: backported type: bug
Projects
None yet
Development

No branches or pull requests

2 participants