On 12/3/19 11:42 AM, Daniil Titov wrote:
Please review the change that makes OperatingSystemMXBean methods return
container specific informationrather than the host based data.
The webrev also takes into account the case when
java.security.AccessControlException exception is thrown
during the initialization of the container subsystem ( e.g. when java.policy doesn’t grant
"read" access to "/proc/self/mountinfo" file).
Instead of failing to access /proc/self/mountinfo, I expect this to wrap
the call with doPrivileged so that it can report the metrics independent
of the security policy. The jdk default security policy should grant
proper permission to do so.
CSR for the spec changes [3] is approved.
Testing: Mach5 tiers1, tiers2, tiers3, tier4, tier5 (including
open/test/hotspot/jtreg/containers/docker), and tier6 tests passed .
[1] Webrev: http://cr.openjdk.java.net/~dtitov/8226575/webrev.02/
[2] Jira issue :https://bugs.openjdk.java.net/browse/JDK-8226575
[3] CSR https://bugs.openjdk.java.net/browse/JDK-8228428
src/java.base/linux/classes/jdk/internal/platform/cgroupv1/Metrics.java
this should wrap the security-sensitive operations with
doPrivileged. jdk.management is trusted and it has all permissions.
src/jdk.management/linux/native/libmanagement_ext/UnixOperatingSystem.c
Formatting nit: line 346-355: JDK native source uses 4-space
identation convention. A space is missing between "if" and "(".
src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java
59 if (limit >= 0 && memLimit >= 0) {
60 return limit - memLimit;
61 }
Under what circumstance that limit or memLimit is < 0? It fallbacks to
return the system's total swap space size - this is not really what it
should report. Is it worth specifying this case?Similarly,
getFreeMemorySize and getTotalMemorySize and getCpuLoad.
getFreeSwapSpaceSize retry for a few times. What special about this
method but not others like getFreeMemorySize?
src/jdk.management/windows/classes/com/sun/management/internal/OperatingSystemImpl.java
There is no strong need to make the deprecated methods as default
methods. If they were default methods, they only need to be implemented
once as opposed to in all OS-specific implementations.
CheckOperatingSystemMXBean.java
System.out.println(String.format(...)) can simply be replaced with
System.out.format.
Mandy
