问题背景

issue 来自 Apache EventMesh,Github 链接:[Enhancement] InterruptedExceptions should never be ignored in the code.[HttpRetryer]

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
dispatcher = new Thread(() -> {
try {
DelayRetryable retryObj;
while (!Thread.currentThread().isInterrupted() && (retryObj = failed.take()) != null) {
final DelayRetryable delayRetryable = retryObj;
pool.execute(() -> {
try {
delayRetryable.retry();
if (retryLogger.isDebugEnabled()) {
retryLogger.debug("retryObj : {}", delayRetryable);
}
} catch (Exception e) {
retryLogger.error("http-retry-dispatcher error!", e);
}
});
}
} catch (Exception e) {
retryLogger.error("http-retry-dispatcher error!", e);
}
}, "http-retry-dispatcher");
dispatcher.setDaemon(true);
log.info("HttpRetryer inited......");

InterruptedExceptions 在代码中不应该被忽略,在这种情况下,简单地记录异常也算作 “忽略”。抛出 InterruptedException 会清除线程的中断状态,所以如果异常处理不当,线程被中断的信息就会丢失。相反,InterruptedExceptions 应该被重新抛出–立即或在清理方法的状态之后–或者通过调用 Thread.interrupt () 来重新中断线程,即使这应该是一个单线程的应用程序。任何其他行为都有可能延迟线程关闭,并丢失线程被中断的信息–可能没有完成其任务。

问题分析

在 #4110 这个 case 中,我以前只在多线程应用中显式处理过 InterruptedExceptions。如果线程在阻塞状态被中断,为了在处理中断后不让后续代码产生错误判断,所以抛出 InterruptedException 的同时会调用 Thread.interrupted () 方法来清除线程的中断状态。

不过 issue 所提到的 dispatcher 是一个单线程应用,如果它在执行 take () 方法时被中断,就会捕获 InterruptedException 异常,然后继续执行异常处理块,此时应该不会丢失线程被中断的信息。但是因为 InterruptedException 是一个 checked exception,如果不对其进行处理,它就会被传播到方法的调用者,有可能会在 EventMeshHTTPServer.java 抛出,从而导致线程的延迟关闭。加之此时也没有及时退出或恢复中断状态,线程可能会继续执行 retry () 方法,进而丢失线程被中断的信息。

但是 issue 里说 InterruptedExceptions 应该被重新抛出,我觉得不合适。这样处理没有重新设置线程的中断状态,也没有向其他开发者传达线程被中断的意图和语义。重新设置中断状态会更好。

当然,想必开发者也清楚这一点~

修改方案

Merged 已合并:[ISSUE #4110] Enhance thread handling of InterruptedException by Pil0tXia · Pull Request #4113 · apache/eventmesh

image-20230618012847499

If a thread is interrupted while in a blocked state, in order to prevent subsequent code from making erroneous judgments after handling the interruption, the InterruptedException is thrown and at the same time, the Thread.interrupted() method is called to clear the thread’s interrupt status.

If the dispatcher is interrupted while executing the take() method, it will catch the InterruptedException exception and continue executing the exception handling block. At this point, the information about the thread being interrupted should not be lost. However, because InterruptedException is a checked exception, if it is not handled, it will be propagated to the method caller and may be thrown in EventMeshHTTPServer.java, resulting in a delayed closure of the thread. Moreover, there is no timely exit or restoration of the interrupt status at this point, so the thread may continue executing the retry() method, thereby losing the information about the thread being interrupted.

However, the issue states that InterruptedExceptions should be re-thrown, but I think this handling is inappropriate. This approach does not reset the thread’s interrupt status nor does it communicate the intention and semantics of the thread being interrupted to other developers. It would be better to reset the interrupt status.

捕获 InterruptedException 异常并重新中断线程:

1
2
3
4
5
6
} catch (Exception e) {
if (e instanceof InterruptedException) {
Thread.currentThread().interrupt();
}
retryLogger.error("http-retry-dispatcher error!", e);
}