On Thu, 26 May 2022 09:42:22 GMT, Maxim Kartashev <[email protected]>
wrote:
>> Following the logic from the comment directly above the changed line, since
>> it doesn't matter which controller we pick, pick any available controller
>> instead of the one called "memory" specifically. This way we are guarded
>> against getting `null` as `anyController`, which is being immediately passed
>> down to `CgroupV2Subsystem.getInstance()` that is unprepared to accept
>> `null` values.
>>
>> It is also worth noting that the previous checks (such as that at line 89)
>> make sure that there exist at least one controller in the map.
>
> Maxim Kartashev has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Removed unnecessary null checks
Changes requested by iklam (Reviewer).
src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java
line 113:
> 111: CgroupInfo anyController = infos.values().iterator().next();
> 112: CgroupSubsystem subsystem =
> CgroupV2Subsystem.getInstance(anyController);
> 113: return new CgroupMetrics(subsystem);
Should add `Objects.requireNonNull(anyController)` and
`Objects.requireNonNull(subsystem)`.
src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemFactory.java
line 116:
> 114: } else {
> 115: CgroupV1Subsystem subsystem =
> CgroupV1Subsystem.getInstance(infos);
> 116: return new CgroupV1MetricsImpl(subsystem);
This shouldn't be changed because the current implementation of
`CgroupV1Subsystem.getInstance(infos)` has a path that returns null.
Maybe that's impossible, because when we call `CgroupV1Subsystem.getInstance`,
we must have at least one v1 subsystem in `infos`. However, that's not related
to this issue. Please fix that in a separate RFE. For example,
`CgroupV1Subsystem.getInstance(infos)` can be changed to throw an exception
instead if return null.
-------------
PR: https://git.openjdk.java.net/jdk/pull/8803
