Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit 9d4ae19

Browse files
committed
Extend T.6 and add TE.9
1 parent 27d85d5 commit 9d4ae19

File tree

1 file changed

+51
-9
lines changed

1 file changed

+51
-9
lines changed

‎README.md

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ Testing
9393
- [Concurrent test workers coordinate their start?](#coordinate-test-workers)
9494
- [There are more test threads than CPUs (if possible for the test)?](#test-workers-interleavings)
9595
- [Assertions in parallel threads and asynchronous code are handled properly?](#concurrent-assert)
96-
- [Checked the result of a `CountDownLatch.await` or `Condition.await` method call?](#check-await)
96+
- [Checked the result of `CountDownLatch.await()`?](#check-await)
9797

9898
Locks
9999
- [Can use some concurrency utility instead of a lock with conditional `wait` (`await`) calls?
@@ -169,6 +169,7 @@ Threads and Executors
169169
](#cf-beware-non-async)
170170
- [Actions are delayed via a `ScheduledExecutorService` rather than `Thread.sleep()`?
171171
](#no-sleep-schedule)
172+
- [Checked the result of `awaitTermination()`?](#check-await-termination)
172173

173174
Parallel Streams
174175
- [Parallel Stream computation takes more than 100us in total?](#justify-parallel-stream-use)
@@ -408,7 +409,7 @@ See [JCIP 2.4] for more information about `@GuardedBy`.
408409
Usage of `@GuardedBy` is especially beneficial in conjunction with [Error Prone](
409410
https://errorprone.info/) tool which is able to [statically check for unguarded accesses to fields
410411
and methods with @GuardedBy annotations](https://errorprone.info/bugpattern/GuardedBy). There is
411-
also an inspection 'Unguarded field access' in IntelliJ IDEA with the same effect.
412+
also an inspection "Unguarded field access" in IntelliJ IDEA with the same effect.
412413

413414
<a name="document-benign-race"></a>
414415
[#](#document-benign-race) Dc.8. If in a thread-safe class some **fields are accessed both from
@@ -805,19 +806,33 @@ thread back to the main test thread and verify it in the end of the test, or to
805806
the boilerplate associated with the first approach.
806807

807808
<a name="check-await"></a>
808-
[#](#check-await) T.6. **Is the result of [`CountDownLatch.await`](
809-
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/CountDownLatch.html#await(long,java.util.concurrent.TimeUnit)),
810-
and [`Condition.await`](
811-
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/locks/Condition.html#await(long,java.util.concurrent.TimeUnit))
809+
[#](#check-await) T.6. **Is the result of [`CountDownLatch.await()`](
810+
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/CountDownLatch.html#await(long,java.util.concurrent.TimeUnit))
812811
method calls checked?** The most frequent form of this mistake is forgetting to wrap
813812
`CountDownLatch.await()` into `assertTrue()` in tests, which makes the test to not actually verify
814813
that the production code works correctly. The absence of a check in production code might cause
815814
race conditions.
816815

816+
Apart from `CountDownLatch.await`, the other similar methods whose result must be checked are:
817+
- [`Lock.tryLock()`](
818+
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/locks/Lock.html)
819+
and `tryAcquire()` methods on [`Semaphore`](
820+
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/Semaphore.html)
821+
and [`RateLimiter`](
822+
https://guava.dev/releases/28.1-jre/api/docs/com/google/common/util/concurrent/RateLimiter.html)
823+
from Guava
824+
- [`Monitor.enter(...)`](https://guava.dev/releases/28.1-jre/api/docs/com/google/common/util/concurrent/Monitor.html) in Guava
825+
- [`Condition.await(...)`](
826+
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/locks/Condition.html#await(long,java.util.concurrent.TimeUnit))
827+
- `awaitTermination()` and `awaitQuiescence()` methods. There is a [separate
828+
item](#check-await-termination) about them.
829+
- [`Process.waitFor(...)`](
830+
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Process.html#waitFor(long,java.util.concurrent.TimeUnit))
831+
817832
It's possible to find these problems using static analysis, e. g. by configuring the "Result of
818-
method call ignored" inspection in IntelliJ IDEA to recognize `CountDownLatch.await` and
819-
`Condition.await`. They are not in the default set of checked methods, so they should be added
820-
manually to the set in the inspection configuration.
833+
method call ignored" inspection in IntelliJ IDEA to recognize `Lock.tryLock()`,
834+
`CountDownLatch.await()` and other methods listed above. They are *not* in the default set of
835+
checked methods, so they should be added manually in the inspection configuration.
821836

822837
<a name="replacing-locks-with-concurrency-utilities"></a>
823838
### Locks
@@ -1357,6 +1372,33 @@ Be cautious, however, about scheduling tasks with affinity to system time or UTC
13571372
beginning of each hour) using `ScheduledThreadPoolExecutor`: it can experience [unbounded clock
13581373
drift](#external-interaction-schedule).
13591374

1375+
<a name="check-await-termination"></a>
1376+
[#](#check-await-termination) TE.9. **The result of `ExecutorService.awaitTermination()` method
1377+
calls is checked?** Calling `awaitTermination()` (or `ForkJoinPool.awaitQuiescence()`) and not
1378+
checking the result makes little sense. If it's actually important to await termination, e. g. to
1379+
ensure a happens-before relation between the completion of the actions scheduled to the
1380+
`ExecutorService` and some actions following the `awaitTermination()` call, or because termination
1381+
means a release of some heavy resource and if the resource is not released there is a noticeable
1382+
leak, then it is reasonable to at least check the result of `awaitTermination()` and log a warning
1383+
if the result is negative, making debugging potential problems in the future easier. Otherwise, if
1384+
awaiting termination really makes no difference, then it's better to not call `awaitTermination()`
1385+
at all.
1386+
1387+
Apart from `ExecutorService`, this item also applies to `awaitTermination()` methods on
1388+
[`AsynchronousChannelGroup`](
1389+
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/nio/channels/AsynchronousChannelGroup.html)
1390+
and [`io.grpc.ManagedChannel`](https://grpc.github.io/grpc-java/javadoc/io/grpc/ManagedChannel.html)
1391+
from [gRPC-Java](https://github.com/grpc/grpc-java).
1392+
1393+
It's possible to find omitted checks for `awaitTermination()` results using [Structural search
1394+
inspection](https://www.jetbrains.com/help/phpstorm/general-structural-search-inspection.html) in
1395+
IntelliJ IDEA with the following pattern:
1396+
```
1397+
$x$.awaitTermination($y,ドル $z$);
1398+
```
1399+
1400+
See also a similar item about [not checking the result of `CountDownLatch.await()`](#check-await).
1401+
13601402
### Parallel Streams
13611403

13621404
<a name="justify-parallel-stream-use"></a>

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /