代码审查清单:Java并发 - Roman Leventov

19-02-03 banq
              

Apache Druid社区,我们目前正在准备一份详细的清单,以便在代码审查期间使用。我决定将清单的一部分作为媒体上的帖子发布,以收集更多关于清单项目的想法。希望有人会发现它在实践中很有用。

顺便说一句,似乎我为代码审查创建项目特定的清单应该是一个强大的想法,但我没有在大型开源项目中看到任何现有的例子。有意思,为什么?

这篇文章包含有关多线程Java代码出现的问题的清单项目。

感谢Marko TopolnikMatko MedenjakChris VestSimon WillnauerBen Manes对本文的评论和贡献。检查表不完整,欢迎提出意见和建议!

1.设计

1.1  如果补丁引入了一个带有并发代码的新子系统,那么在补丁描述中是否需要合并并发化?是否讨论了可以简化代码并发模型的替代设计方法(参见下一个项目)?

1.2  是否可以应用一个或多个设计模式(其中一些在下面列出)以显着简化代码的并发模型,同时不会明显影响其他质量方面,如整体简单性,效率,可测试性,可扩展性等?

不变性/快照服务。当某个状态应该更新时,会创建,发布和使用新的不可变对象(或可变对象中的快照),而某些并发线程仍可能使用较旧的副本或快照。参见CopyOnWriteArrayList,CopyOnWriteArraySet,持久数据结构

分而治之。工作分为几个独立处理的部分,每个部分都在一个线程中。然后结合处理结果。Parallel Streams或ForkJoinPool可用于应用此模式。

生产者-消费者。工作线程通过队列在工作线程之间传输。参见[JCIP 5.3],本清单中的项目6.1,CSPSEDA

实例限制。某些根类型的对象封装了一些复杂的分层子状态。Root对象主要负责从多个线程访问和修改子状态的安全性。换句话说,组合对象是同步的,而不是组成同步对象。见[JCIP 4.2,10.1.3,10.1.4]。

线程/任务/串行线程限制。通过自上而下的传递参数或ThreadLocal使得某些状态成为本地状态。见[JCIP 3.3]。任务限制是线程限制思想的变体,与分而治之模式结合使用。它通常以lambda捕获的“context”参数或每个线程任务对象中的字段的形式出现。串行线程限制是生产者 - 消费者模式的线程限制思想的扩展,参见[JCIP 5.3.2]。

2. 文档

2.1 对于具有线程安全标志的每个类,方法和字段,例如synchronized关键字,volatile字段上的修饰符,使用任何类java.util.concurrent.*或第三方并发原语或并发集合,需要实现他们的Javadoc注释包括:

  • 线程安全的理由:是否解释了为什么特定的类,方法或字段必须是线程安全的?
  • 并发控制流文档:它是从什么方法和上下文中枚举线程安全类的每个特定方法被调用的线程(执行程序,线程池)的内容?

2.2 如果补丁引入了一个使用线程或线程池的新子系统,那么是否有线程模型的高级描述,子系统的并发控制流(或数据流),例如在包中的Javadoc注释package-info.java或对于子系统的主类?添加新线程或线程池或从系统中删除一些旧线程时,这些描述是否保持最新?

线程模型的描述包括子系统中创建和管理的线程和线程池的枚举,子系统中使用的外部池(例如ForkJoinPool.commonPool()),它们的大小和其他重要特性(如线程优先级)以及托管线程的生命周期和线程池。

并发控制流的高级描述应该是概述,并将各个类的并发控制流文档联系在一起,请参阅上一项。如果使用生产者 - 消费者模式,则并发控制流是微不足道的,应该记录数据流。

描述线程模型和控制/数据流极大地提高了系统的可维护性,因为在没有描述或图表的情况下,开发人员花费大量时间和精力在他们的脑海中创建和刷新这些模型。放下模型也有助于发现瓶颈和简化设计的方法(参见第1.2项)。

2.3 对于作为公共API的一部分或项目的扩展API的类和方法:是否在它们的Javadoc注释中指定它们是否是(或者在扩展中为子类化设计的接口和抽象类的情况下,它们是否应实现为)不可变,线程安全或不是线程安全的?对于(或应该实现为)线程安全的类和方法,是否可以精确地记录可以从多个线程同时调用它们的其他方法(或它们自己)?另见[EJ Item 82]和[JCIP 4.5]。

2.4 对于使用某些并发设计模式的子系统,类,方法和字段,可以是高级别(例如本清单中第1.2项中提到的那些)或低级别(例如双重检查锁定,请参阅此核对表中的第8节) ):在各个子系统,类,方法和字段的设计或实现注释中是否发现了使用的并发模式?这有助于读者更快地理解代码。

2.5 ConcurrentHashMap和ConcurrentSkipListMap对象存储在ConcurrentHashMap或者ConcurrentSkipListMap或ConcurrentMap类型字段和变量的中,不仅仅是Map?

这很重要,因为在如下代码中:

ConcurrentMap<String, Entity> entities = getEntities();
if (!entities.containsKey(key)) {
  entities.put(key, entity);
} else {
  ...
}

很明显可能存在竞争条件,如果并发线程在containsKey()和put()之间同时调用,一个实体会被两个并发线程放入Map中。

如果entities变量的类型只是Map<String, Entity>它不那么明显,读者可能会认为这只是稍微不理想的代码并且通过。

可以将此建议转换 IntelliJ IDEA中的检查

banq注:由于过于冗长复杂,且可能过于教条,点击标题参考原文