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

ConcurrentHashMapUtils无法解决jdk8的循环bug #11986

Closed
looly opened this issue Apr 2, 2023 · 5 comments · Fixed by #11987
Closed

ConcurrentHashMapUtils无法解决jdk8的循环bug #11986

looly opened this issue Apr 2, 2023 · 5 comments · Fixed by #11987
Labels
type/bug Bugs to being fixed

Comments

@looly
Copy link
Contributor

looly commented Apr 2, 2023

Environment

  • Dubbo version: 3.2, 3.3
  • Operating System version: Windows11
  • Java version: jdk8

Steps to reproduce this issue

ConcurrentHashMapUtils 中提供的computeIfAbsent方法没有解决jdk8的循环问题,复现代码如下:

final ConcurrentHashMap<String,Integer> map=new ConcurrentHashMap<>();
// // map.computeIfAbsent("AaAa", key->map.computeIfAbsent("BBBB",key2->42));
ConcurrentHashMapUtils.computeIfAbsent(map, "AaAa", key->map.computeIfAbsent("BBBB",key2->42));

Hutool 中的解决方案是:

V value = map.get(key);
if (null == value) {
	map.putIfAbsent(key, mappingFunction.apply(key));
	value = map.get(key);
}
return value;
@looly looly added the type/bug Bugs to being fixed label Apr 2, 2023
@looly
Copy link
Contributor Author

looly commented Apr 2, 2023

但是有issue反馈说这种方式获取的value有可能为null,我不确定这种写法是否存在问题。

issue中提出的替代方案是:

V value = map.get(key);
if (null == value) {
	value = mappingFunction.apply(key);
	final V res = map.putIfAbsent(key, mappingFunction.apply(key));
	if(null != res){
		// issues#I6RVMY
		// 如果旧值存在,说明其他线程已经赋值成功,putIfAbsent没有执行,返回旧值
		return res;
	}
	// 如果旧值不存在,说明赋值成功,返回当前值
}
return value;

@AlbumenJ
Copy link
Member

AlbumenJ commented Apr 2, 2023

ConcurrentHashMapUtils 这个主要是解决性能的问题

@AlbumenJ
Copy link
Member

AlbumenJ commented Apr 2, 2023

死锁这个问题可以提个 PR 改下

@looly
Copy link
Contributor Author

looly commented Apr 2, 2023

看到原来的issue和PR:

#11326

#11294

这个PR是为了解决 https://bugs.openjdk.org/browse/JDK-8161372 这个问题,但这本质上是个死锁问题。

@AlbumenJ 没有提PR就是因为两个解决方案我都不能保证逻辑是正确的,需要review验证,这也是困扰我的地方。

稍后提个PR。

looly added a commit to looly/dubbo that referenced this issue Apr 2, 2023
looly added a commit to looly/dubbo that referenced this issue Apr 2, 2023
looly added a commit to looly/dubbo that referenced this issue Apr 2, 2023
@looly
Copy link
Contributor Author

looly commented Apr 2, 2023

PR see: #11986

AlbumenJ added a commit that referenced this issue May 10, 2023
* fix #11986

* fix #11986

* fix #11986

* change test at jdk8

* seprate test for jdk8 and jdk17

* seprate jre version test

* change comment to english

* add null check for v

---------

Co-authored-by: Albumen Kevin <jhq0812@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Bugs to being fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants