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

Alias resolution in SimpleAliasRegistry depends on hash codes of alias values #32024

Closed
sbrannen opened this issue Jan 14, 2024 · 2 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@sbrannen
Copy link
Member

Overview

As discussed in commit 5d309d5, alias resolution in SimpleAliasRegistry.resolveAliases() currently depends on the iteration order of map entries in the aliasMap which is a ConcurrentHashMap.

In other words, the order in which aliases are processed depends on their hash code values.

This results in different behavior for the same set of logical alias pairs if the names of some of the aliases are changed in such a way that their hash codes result in a different iteration order for the aliasMap.

For example, given an existing, working application that relies on aliases and placeholder replacement for such aliases, simply changing the name of one of those aliases may result in failure to start the application.

Possible Solutions

Using a LinkedHashMap for aliasMap and aliasCopy ensures alias processing in the order in which aliases were registered.

However, we currently use a ConcurrentHashMap for aliasMap, so we would need to wrap that in Collections.synchronizedMap(). That works, but we may not want to use a synchronized LinkedHashMap for the aliasMap in general.

Thus, another possibility proposed by @jhoeller is to track the names of registered aliases separately:

Along the lines of this.beanDefinitionNames vs. this.beanDefinitionMap in DefaultListableBeanFactory, we could preserve the iteration order for the keys separately and just use it for the forEach loop in resolveAliases(). That would still be a separate data structure, increasing the footprint, but only for the keys then.

Related Issues

@sbrannen sbrannen added type: bug A general bug in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 14, 2024
@sbrannen sbrannen self-assigned this Jan 14, 2024
@sbrannen sbrannen added this to the 6.1.x milestone Jan 14, 2024
@sbrannen sbrannen changed the title Alias resolution in SimpleAliasRegistry depends on hash codes of alias String values Alias resolution in SimpleAliasRegistry depends on hash codes of alias values Jan 14, 2024
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Jan 14, 2024
@sbrannen
Copy link
Member Author

Current proposal for a fix can be viewed in the following feature branch.

main...sbrannen:spring-framework:issues/gh-32024-SimpleAliasRegistry-consistent-alias-processing-order

sbrannen added a commit to sbrannen/spring-framework that referenced this issue Jan 14, 2024
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Jan 14, 2024
@sbrannen
Copy link
Member Author

sbrannen commented Jan 14, 2024

For example, given an existing, working application that relies on aliases and placeholder replacement for such aliases, simply changing the name of one of those aliases may result in failure to start the application.

For anyone reading this issue and wondering how that can happen, the simplest analogy I can come up with is the standard swap algorithm in computer science.

@Test
void swap() {
	String aliasX = "bean1";
	String aliasY = "bean2";

	String temp = aliasX;
	aliasX = aliasY;
	aliasY = temp;

	assertThat(aliasX).isEqualTo("bean2");
	assertThat(aliasY).isEqualTo("bean1");
}

The swap works if aliasX is processed before aliasY, but if we change the order of the aliasX = aliasY; and aliasY = temp; statements, then aliasX and aliasY will both have the value of bean1.

And that's effectively what can happen currently in SimpleAliasRegistry.

@sbrannen sbrannen modified the milestones: 6.1.x, 6.2.0-M1 Jan 14, 2024
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Jan 19, 2024
@bclozel bclozel changed the title Alias resolution in SimpleAliasRegistry depends on hash codes of alias values Alias resolution in SimpleAliasRegistry depends on hash codes of alias values Feb 14, 2024
@sbrannen sbrannen changed the title Alias resolution in SimpleAliasRegistry depends on hash codes of alias values Alias resolution in SimpleAliasRegistry depends on hash codes of alias values Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

1 participant