Doug Lea在J.U.C包里面写的BUG又被网友发现了

这是why的第 69 篇原创文章

DougLea在J.U.C包里面写的BUG又被网友发现了

BUG描述

一个编号为 8073704 的 JDK BUG,将串联起我的这篇文章。

也就是下面的这个链接。

https://bugs.openjdk.java.net/browse/JDK-8073704

这个 BUG 在 JDK 9 版本中进行了修复。也就是说,如果你用的 JDK 8,也许会遇到这样的问题。

先带大家看看这个问题是怎么样的:

DougLea在J.U.C包里面写的BUG又被网友发现了

这个 BUG 说: FutureTask.isDone 方法在任务还没有完成的时 就会返回 true。

DougLea在J.U.C包里面写的BUG又被网友发现了

可以看到,这是一个 P4 级别(优先级不高)的 BUG,这个 BUG 也是分配给了 Doug Lea,因为 FutureTask 就是他写的:

DougLea在J.U.C包里面写的BUG又被网友发现了

响应了国家政策:谁污染,谁治理。

这个 BUG 的作者 Martin 老哥是这样描述的:

DougLea在J.U.C包里面写的BUG又被网友发现了

下面我会给大家翻译一下他要表达的东西。

但是在翻译之前,我得先做好背景铺垫,以免有的朋友看了后一脸懵逼。

如果要懂他在说什么,那我必须得再给你看个图片,这是 FutureTask 的文档描述:

DougLea在J.U.C包里面写的BUG又被网友发现了

看 Martin 老哥提交的 BUG 描述的时候,得对照着状态图和状态对应的数字来看。

他说 FutureTask#isDone 方法现在是这样写的:

DougLea在J.U.C包里面写的BUG又被网友发现了

他觉得从源码来看,是只要当前状态不等于 NEW(即不等于0)则返回 true,表示任务完成。

DougLea在J.U.C包里面写的BUG又被网友发现了

他觉得应该是这样写:

DougLea在J.U.C包里面写的BUG又被网友发现了

这样写的目的是除了判断了 NEW 状态之外, 还判断了两个中间状态:COMPLETING 和 INTERRUPTING。

那么除去上面的三个状态之外呢,就只剩下了这四个状态:

DougLea在J.U.C包里面写的BUG又被网友发现了

这四个状态可以代表一个任务的最终状态。

当然,他说上面的代码还有优化空间,比如下面这样,代码少了,但是理解起来也得多转个弯:

DougLea在J.U.C包里面写的BUG又被网友发现了

state>COMPLETING,满足这个条件的状态只有下面这几种:

DougLea在J.U.C包里面写的BUG又被网友发现了

而这几种中,只有 INTERRUPTING 是一个中间态,所以他用后面的 != 排除掉了。

这样就是代码简洁了,但是理解起来多转个小弯。但是这两段代码表示的含义是一模一样的。

好了,关于这个 BUG 的描述就是这样的。

汇总为一句话就是,这个 Martin 老哥认为:

FutureTask.isDone 方法在任务还没有完成的时候,比如还是 COMPLETING 和 INTERRUPTING 的时候就会返回 true,这样是不对的。这就是 BUG。

仅从 isDone 源码中那段 status != NEW 的代码,我认为这个 Martin 老哥说的确实没有问题。因为确实有两个中间态,这段源码中是没有考虑的。

接下来,我们就围绕着这个问题进行展开,看看各位大神的讨论。

DougLea在J.U.C包里面写的BUG又被网友发现了

展开讨论

首先,第一个发言的哥们是 Pardeep,是在这个问题被提出的 13 天之后:

DougLea在J.U.C包里面写的BUG又被网友发现了

我没有太 get 到这个哥们回答的点是什么啊。

他说:我们应该去看一下 isDone 方法的描述。

DougLea在J.U.C包里面写的BUG又被网友发现了

描述上说: 如果一个任务已完成,调用这个方法则返回true。而完成除了是正常完成外,还有可能是任务异常或者任务取消导致的完成,这些都算完成。

我觉得他的这个回答和问题有点对不上号,感觉是答非所问。

就当他抛出了一个关于 isDone 方法的知识点吧。

DougLea在J.U.C包里面写的BUG又被网友发现了

三天后,第二个发言的哥们叫做 Paul,他的观点是这样的:

DougLea在J.U.C包里面写的BUG又被网友发现了

首先,他说我们不需要检查 INTERRUPING 这个中间状态。

因为如果一个任务处于这个状态,那么获取结果的时候一定是抛出 CancellationException。

叫我们看看 isCancelled 方法和 get 方法。

那我们先看看 isCancelled 方法:

DougLea在J.U.C包里面写的BUG又被网友发现了

直接判断了状态是否大于等于 CANCELLED,也就是判断了状态是否是这三种中的一个:

DougLea在J.U.C包里面写的BUG又被网友发现了

判断任务是否取消(isCancelled)的时候,并没有对 INTERRUPING 这个中间状态做特殊处理。

按照这个逻辑,那么判断任务是否完成(isDone)的时候,也不需要对 INTERRUPING 这个中间状态做特殊处理。

接着,我们看看 get 方法。

get 方法最终会调用这个 report 方法:

DougLea在J.U.C包里面写的BUG又被网友发现了

如果变量 s (即状态)是 INTERRUPING (值是 5),那么是大于 CANCELLED (值是 4)状态 ,则抛出 CancellationException (CE)异常。

所以,他觉得对于 INTERRUPING 状态没有必要进行检测。

因为如果此状态下,你调用 isCancelled 方法,那么会告诉你任务取消了。

如果你直接调用 get 方法,会抛出 CE 异常。

所以,综上所述,我认为 Paul 这个哥们的逻辑是这样的:

我们作为使用者,最终都会调用 get 方法来获取结果,假设在调用 get 方法之前。我们用 isCancelled 或者 isDone 判断了一下任务的状态。

如果当前状态好死不死的就是 INTERRUPING 。那么调用 isCancelled 返回 true,那按照正常逻辑,是不会继续调用 get 方法的。

如果调用的是 isDone ,那么也返回 true,就会去调用 get 方法。

在 get 方法这里保证了,就算当前处于 INTERRUPING 中间态,程序抛出 CE 异常就可以了。

因此,Paul 认为如果没有必要检测 INTERRUPING 状态的话,那么我们就可以把代码从:

DougLea在J.U.C包里面写的BUG又被网友发现了

简化为:

DougLea在J.U.C包里面写的BUG又被网友发现了

但是,这个哥们还说了一句话来兜底。

他说:Unless i have missed something subtle about the interactions

除非我没有注意到一些非常小的细节问题。

你看,说话的艺术。话都被他一个人说完了。

DougLea在J.U.C包里面写的BUG又被网友发现了

好了,Paul 同学发言完毕了。42 分钟之后,一个叫 Chris 的小老弟接过了话筒,他这样说的:

DougLea在J.U.C包里面写的BUG又被网友发现了

我觉得吧,Paul 说的挺有道理的,我赞成他的建议。

DougLea在J.U.C包里面写的BUG又被网友发现了

但是吧,我也觉得我们在讨论的是一个非常细节,非常小的问题,我不知道,就算现在这样写,会导致任何问题吗?

写到这里,先给大家捋一下:

  • Martin 老哥提出 BUG 说 FutureTask#isDone 方法没有判断 INTERRUPING 和 COMPLETING 这个两个中间状态是不对的。
  • Paul 同学说,对于 INTERRUPING 这个状态,可以参照 isCancelled 方法,不需要做特殊判断。
  • Chris 小老弟说 Paul 同学说的对。

于是他们觉得 isDone 方法应该修改成这样:

DougLea在J.U.C包里面写的BUG又被网友发现了

所以,现在只剩下一个中间状态是有争议的了:COMPLETING 。

对于剩下的这个中间状态,一位叫做 David 的靓仔,在三小时后发表了自己的意见:

DougLea在J.U.C包里面写的BUG又被网友发现了

他上来就是一个暴击,直截了当的说:我认为在座的各位都是垃圾。

DougLea在J.U.C包里面写的BUG又被网友发现了

好吧,他没有这样说。

其实他说的是:我认为没有必要做任何改变。

COMPLETING 状态是一个转瞬即逝的过渡状态,它代表我们已经有最终状态了,但是在设置最终状态开始和结束的时间间隙内有一个瞬间状态,它就是 COMPLETING 状态。

其实你是可以通过 get 方法知道任务是否是完成了,通过 get 方法你可以获得最终的正确答案。

因为 COMPLETING 这个转瞬即逝的过渡状态是不会被程序给检测到的。

David 靓仔的回答在两个半小时候得到了大佬的肯定:

DougLea在J.U.C包里面写的BUG又被网友发现了

Doug Lea 说:现在源码里面是故意这样写的,原因就是 David 这位靓仔说的,我写的时候就是这样考虑过的。

DougLea在J.U.C包里面写的BUG又被网友发现了

另外,我觉得这个 BUG 的提交者自己应该解释我们为什么需要修改这部分代码。

其实 Doug 的言外之意就是:你说这部分有问题,你给我举个例子,别只是整理论的,你弄点代码给我看看。

DougLea在J.U.C包里面写的BUG又被网友发现了

半小时之后,这个 BUG 的提交者回复了:

DougLea在J.U.C包里面写的BUG又被网友发现了

intentional 知道是啥意思不?

,我又得兼职教英语了:

DougLea在J.U.C包里面写的BUG又被网友发现了

他说:哦,原来是故意的呀。

这句话,你用不同的语气可以读出不同的含义。

我这里倾向于他觉得既然 Doug 当初写这段代码的时候考虑到了这点,他分析之后觉得自己这样写是没有问题的,就这样写了。

好嘛,前面说 INTERRUPING 不需要特殊处理,现在说 COMPLETING 状态是检测不到的。

那就没得玩了。

DougLea在J.U.C包里面写的BUG又被网友发现了

事情现在看起来已经是被定性了,那就是不需要进行修改。

但是就在这时 Paul 同学杀了个回马枪,应该也是前面的讨论激发了他的思路,你不是说检测不出来吗,你不是说 get 方法可以获得最终的正确结果吗?

那你看看我这段代码是什么情况:

DougLea在J.U.C包里面写的BUG又被网友发现了

代码是这样的,大家可以直接粘贴出来,在 JDK 8/9 环境下分别运行一下:

publicstaticvoidmain(String[]args)throwsException{

AtomicReference<FutureTask<Integer>>a=newAtomicReference<>();
Runnabletask=()->{
while(true){
FutureTask<Integer>f=newFutureTask<>(()->1);
a.set(f);
f.run();
}
};

Supplier<Runnable>observe=()->()->{
while(a.get()==null);

intc=0;
intic=0;
while(true){
c++;
FutureTask<Integer>f=a.get();
while(!f.isDone()){}
try{
/*
Settheinterruptflagofthisthread.
Thefuturereportsitisdonebutinsomecasesacallto
"get"willresultinanunderlyingcallto"awaitDone"if
thestateisobservedtobecompleting.
"awaitDone"checksifthethreadisinterruptedandifso
throwsanInterruptedException.
*/
Thread.currentThread().interrupt();
f.get();
}
catch(ExecutionExceptione){
thrownewRuntimeException(e);
}
catch(InterruptedExceptione){
ic++;
System.out.println("InterruptedExceptionobservedwhenisDone()==true"+c+""+ic+""+Thread.currentThread());
}
}
};

CompletableFuture.runAsync(task);
Stream.generate(observe::get)
.limit(Runtime.getRuntime().availableProcessors()-1)
.forEach(CompletableFuture::runAsync);

Thread.sleep(1000);
System.exit(0);
}

先看一下这段代码的核心逻辑:

DougLea在J.U.C包里面写的BUG又被网友发现了

首先标号为 ① 的地方是两个计数器,c 代表的是第一个 while 循环的次数,ic 代表的是抛出 InterruptedException(IE) 的次数。

标号为 ② 的地方是判断当前任务是否是完成状态,如果是,则继续往下。

标号为 ③ 的地方是先中断当前线程,然后调用 get 方法获取任务结果。

标号为 ④ 的地方是如果 get 方法抛出了 IE 异常,则在这里进行记录,打印日志。

需要注意的是,如果打印日志了,说明了一个问题:

前面明明 isDone 方法返回 true 了,说明方法执行完成了。但是我调用 get 方法的时候却抛出了 IE 异常?

这你怕是有点说不通吧!

JDK 8 的运行结果我给大家截个图。

DougLea在J.U.C包里面写的BUG又被网友发现了

这个异常是在哪里被抛出来的呢?

awaitDone 方法的入口处,就先检查了当前线程是否被中断,如果被中断了,那么抛出 IE 异常:

DougLea在J.U.C包里面写的BUG又被网友发现了

而代码怎么样才能执行到 awaitDone 方法呢?

DougLea在J.U.C包里面写的BUG又被网友发现了

任务状态是小于等于 COMPLETING 的时候。

在示例代码中,前面的 while 循环中的 isDone 方法已经返回了 true,说明当前状态肯定不是 NEW。

那么只剩下个什么东西了?

就只有一个 COMPLETING 状态了。

小样,这不就是监测到了吗?

DougLea在J.U.C包里面写的BUG又被网友发现了

在这段示例代码出来后的第 8 个小时,David 靓仔又来说话了:

DougLea在J.U.C包里面写的BUG又被网友发现了

他要表达的意思,我理解的是这样的:

在 j.u.c 包里面,优先检查线程中断状态是很常见的操作,因为相对来说,会导致线程中断的地方非常的少。

但是不能因为少,我们就不检查了。

我们还是得对其进行了一个优先检查,告知程序当前线程是否发生了中断,即是否有继续往下执行的意义。

但是,在这个场景中,当前线程中断了,但并不能表示 Future 里面的 task 任务的完成情况。这是两个不相关的事情。

即使当前线程中断了,但是 task 任务仍然可以继续完成。但是执行 get 方法的线程被中断了,所以可能会抛出 InterruptedException。

因为,他给出的解决建议是:

可以选择优先返回结果,在 awaitDone 方法的循环中把检查中断的代码挪到后面去。

五天之后,之前 BUG 的提交者 Martin 同学又来了:

DougLea在J.U.C包里面写的BUG又被网友发现了

他说他改变主意了。

改变什么主意了?他之前的主意是什么?

在 Doug 说他是故意这样写的之后,Martin 说:

It's intentional。哦,原来是故意的呀。

那个时候他的主意就是:大佬都说了,这样写是考虑过的,肯定没有问题。

现在他的主意是: 如果 isDone 方法返回了 true,那么 get 方法应该明确的返回结果值,而不会抛出 IE 异常。

需要注意的是,这个时候对于 BUG 的描述已经发生变化了。

从“ FutureTask.isDone 方法在任务还没有完成的时候就会返回 true ”变成了“ 如果 isDone 方法返回了 true,那么 get 方法应该明确的返回结果值,而不会抛出 IE 异常 ”。

然后 David 靓仔给出了一个最简单的解决方案:

DougLea在J.U.C包里面写的BUG又被网友发现了

最简单的解决方案就是先检查状态,再检查当前线程是否中断。

然后,这个 BUG 由 Martin 同学进行了修复:

DougLea在J.U.C包里面写的BUG又被网友发现了

修复的代码可以先不看,下面一小节我会给大家做个对比。

他修复的同时还小心翼翼的要求 Doug 祝福他,为他站个台。

最后,Martin 同学说他已经提交给了 jsr166,预计在 JDK 9 版本进行修复。

出于好奇,我在 JDK 的源码中搜索了一下 Martin 同学的名字,本以为是个青铜,没想到是个王者,失敬失敬:

DougLea在J.U.C包里面写的BUG又被网友发现了

代码对比

既然说在 JDK 9 中对该 BUG 进行了修复,那么带大家对比一下 JDK 9/8 的代码。

java.util.concurrent.FutureTask#awaitDone:

DougLea在J.U.C包里面写的BUG又被网友发现了

可以看到,JDK 9 把检查是否中断的操作延后了一步。

代码修改为这样后,把之前的那段示例代码放到 JDK 9 上面跑一下,你会惊奇的发现,没有抛出异常了。

因为源码里面判断 COMPLETING 的操作在判断线程中断标识之前:

DougLea在J.U.C包里面写的BUG又被网友发现了

我想就不需要我再过多解释了吧。

然后多说一句 JDK 9 现在的 FutureTask#awaitDone 里面有这样的一行注释:

DougLea在J.U.C包里面写的BUG又被网友发现了

它说:isDone 方法已经告诉使用者任务已经完成了,那么调用 get 方法的时候我们就不应该什么都不返回或者抛出一个 IE 异常。

这行注释想要表达的东西,就是上面一小节的 BUG 里面我们在讨论的事情。写这行注释的人,就是 Martin 同学。

当我了解了这个 BUG 的来龙去脉之后,又突然间在 JDK 9 的源码里面看到这个注释的时候,有一种很神奇的感觉。

就是一种源码说:you feel me?

我马上心领神会:I get you。

挺好。

DougLea在J.U.C包里面写的BUG又被网友发现了

虚假唤醒

在 JDK 9 的注释里面还有这个词汇:

DougLea在J.U.C包里面写的BUG又被网友发现了

spurious wakeup,虚假唤醒。

如果你之前不知道这个东西的存在,那么恭喜你,又 get 到了一个你基本上用不到的知识点。

除非你自己需要在代码中用到 wait、notify 这样的方法。

哦,也不对,面试的时候可能会用到。

“虚假唤醒”是怎么一回事呢,我给你看个例子:

java.lang.Thread#join(long) 方法:

DougLea在J.U.C包里面写的BUG又被网友发现了

这里为什么要用 while 循环,而不是直接用 if 呢?

因为循环体内有调用 wait 方法。

为什么调用了 wait 方法就必须用 while 循环呢?

别问,问就是防止虚假唤醒。

看一下 wait 方法的 javadoc:

DougLea在J.U.C包里面写的BUG又被网友发现了

一个线程能在没有被通知、中断或超时的情况下唤醒,也即所谓的“虚假唤醒”,虽然这点在实践中很少发生,但是程序应该循环检测导致线程唤醒的条件,并在条件不满足的情况下继续等待,来防止虚假唤醒。

所以,建议写法是这样的:

DougLea在J.U.C包里面写的BUG又被网友发现了

在 join 方法中,isAlive 方法就是这里的 condition does not hold。

在《Effective Java》一书中也有提到“虚假唤醒”的地方:

DougLea在J.U.C包里面写的BUG又被网友发现了

书中的建议是: 没有理由在新开发的代码中使用 wait、notify 方法,即使有,也应该是极少了,请多使用并发工具类。

再送你一个面试题: 为什么 wait 方法必须放在 while 循环体内执行?

现在你能回答的上来这个问题了吧。

关于“虚假唤醒”就说这么多,有兴趣的同学可以再去仔细了解一下。

Netty的一个BUG

好好的说着 JDK 的 FutureTask 呢,怎么突然转弯到 Netty 上了?

因为 Netty 里面,其核心的 Future 接口实现中,犯了一个基本的逻辑错误,在实现 cancel 和 isDone 方法 违反了 JDK 的约定。

这是一个让 Netty 作者也感到惊讶的错误。

先看看 JDK Future 接口中,对于 cancel 方法的说明:

https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/Future.html

DougLea在J.U.C包里面写的BUG又被网友发现了

文档的方法说明上说: 如果调用了 cancel 方法,那么再调用 isDone 将永远返回 true。

看一下这个测试代码:

DougLea在J.U.C包里面写的BUG又被网友发现了

可以看到,在调用了 cancel 方法后,再次调用 isDone 方法,返回的却是 false。

这个点我是很久之前在知乎的这篇文章上看到的,和本文讨论的内容有一点点相关度,我就又翻了出来,多说了一嘴。

有兴趣的可以看看:

《一个让Netty作者也感到惊讶的错误》

https://zhuanlan.zhihu.com/p/34609401