RFR: 8273597: Rectify Thread::is_ConcurrentGC_thread() [v2]
Per Liden
pliden at openjdk.java.net
Mon Sep 13 12:07:17 UTC 2021
> `Thread::is_ConcurrentGC_thread()` behaves differently to all other `Thread::is_xxx_thread()` functions, in the sense that it doesn't directly map to a distinct `Thread` sub-class. Instead, `is_ConcurrentGC_thread()` can today return true for both `ConcurrentGCThread` and `GangWorker`. These two classes have no super/sub-class relation. This is confusing and and potentially dangerous.
>
> It would be reasonable to think that code like this would be correct:
>
>
> if (thread->is_ConcurrentGC_thread()) {
> conc_thread = static_cast<ConcurrentGCThread*>(thread);
> ...
> }
>
>
> but it's not, since we might try to cast a `GangWorker` to a `ConcurrentGCThread`. And again, these two classes have no super/sub-class relation.
>
> I propose that we clean this up, so that `is_ConcurrentGCThread()` only returns true for threads inheriting from `ConcurrentGCThread`. The main side-effect is that a handful of asserts need to be adjusted. In return, the code example above would become legal, and we can also remove some cruft from `WorkGang`/`GangWorker`.
Per Liden has updated the pull request incrementally with two additional commits since the last revision:
- Fix constructor call
- Add ConcurrentGCThread::cast()
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/5463/files
- new: https://git.openjdk.java.net/jdk/pull/5463/files/91e70702..fd645dd3
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=5463&range=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=5463&range=00-01
Stats: 9 lines in 3 files changed: 6 ins; 1 del; 2 mod
Patch: https://git.openjdk.java.net/jdk/pull/5463.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/5463/head:pull/5463
PR: https://git.openjdk.java.net/jdk/pull/5463
More information about the shenandoah-dev
mailing list