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

本文由简悦 SimpRead 转码，原文地址 www.toutiao.com

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

这是 why 的第 69 篇原创文章

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

也就是下面的这个链接。

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

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

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

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

可以看到，这是一个 P4 级别（优先级不高）的 BUG，这个 BUG 也是分配给了 Doug Lea，因为 FutureTask 类就是他写的：

响应了国家政策：谁污染，谁治理。

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

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

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

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

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

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

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

他觉得应该是这样写：

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

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

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

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

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

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

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

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

汇总为一句话就是，这个 Martin 老哥认为：

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

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

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

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

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

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

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

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

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

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

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

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

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

那我们先看看 isCancelled 方法：

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

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

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

接着，我们看看 get 方法。

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

如果变量 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 状态的话，那么我们就可以把代码从：

简化为：

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

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

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

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

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

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

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

写到这里，先给大家捋一下：

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

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

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

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

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

好吧，他没有这样说。

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

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

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

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

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

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

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

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

半小时之后，这个 BUG 的提交者回复了：

intentional 知道是啥意思不？

害，我又得兼职教英语了：

他说：哦，原来是故意的呀。

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

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

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

那就没得玩了。

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

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

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

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

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50


public static void main(String[] args) throws Exception {

        AtomicReference<FutureTask<Integer>> a = new AtomicReference<>();
        Runnable task = () -> {
            while (true) {
                FutureTask<Integer> f = new FutureTask<>(() -> 1);
                a.set(f);
                f.run();
            }
        };

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

            int c = 0;
            int ic = 0;
            while (true) {
                c++;
                FutureTask<Integer> f = a.get();
                while (!f.isDone()) {}
                try {
                    /*
                    Set the interrupt flag of this thread.
                    The future reports it is done but in some cases a call to
                    "get" will result in an underlying call to "awaitDone" if
                    the state is observed to be completing.
                    "awaitDone" checks if the thread is interrupted and if so
                    throws an InterruptedException.
                     */
                    Thread.currentThread().interrupt();
                    f.get();
                }
                catch (ExecutionException e) {
                    throw new RuntimeException(e);
                }
                catch (InterruptedException e) {
                    ic ++;
                    System.out.println("InterruptedException observed when isDone() == 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);
    }

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

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

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

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

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

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

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

这你怕是有点说不通吧！

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

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

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

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

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

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

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

就只有一个 COMPLETING 状态了。

小样，这不就是监测到了吗？

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

他要表达的意思，我理解的是这样的：

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

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

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

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

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

因为，他给出的解决建议是：

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

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

他说他改变主意了。

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

在 Doug 说他是故意这样写的之后，Martin 说：

It’s intentional。哦，原来是故意的呀。

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

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

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

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

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

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

然后，这个 BUG 由 Martin 同学进行了修复：

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

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

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

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

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

java.util.concurrent.FutureTask#awaitDone：

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

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

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

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

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

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

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

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

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

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

挺好。

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

spurious wakeup，虚假唤醒。

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

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

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

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

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

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

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

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

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

看一下 wait 方法的 javadoc：

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

所以，建议写法是这样的：

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

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

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

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

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

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

好好的说着 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

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

看一下这个测试代码：

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

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

有兴趣的可以看看：

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

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