RFR: 8226575: OperatingSystemMXBean should be made container aware

[Mandy Chung]

On 12/9/19 10:51 AM, Daniil Titov wrote:
> Hi Mandy and Bob,
>
>> Why did you not change the exception caught in SubSystem.java:getStringValue to PrivilegedActionException from IOException
>> so it’s consistent with the other get functions?
> In this method both Files.newBufferedReader and return bufferedReader.readLine could throw IOException so for simplicity I just put
> the whole code block in doPrivileged. On the other side I don't believe that BufferedReader.readline() requires  FilePermission checks ( and tests proved that)
> so we could change this implementation to the following:
>
>      public static String getStringValue(SubSystem subsystem, String parm) {
>          if (subsystem == null) return null;
>
>         try (BufferedReader bufferedReader =
>                       AccessController.doPrivileged((PrivilegedExceptionAction<BufferedReader>) () -> {
>                           return Files.newBufferedReader(Paths.get(subsystem.path(), parm));
>                       })) {
>              return bufferedReader.readLine();
>          } catch (PrivilegedActionException | IOException  e) {
>              return null;
>          }
>      }
>
> Could you please advise are you OK with it or you would like to proceed with the approach Mandy suggested to unwrap
> PrivilegedActionException exception and throw the cause instead?
>

I think it's simpler to read and understand if the doPrivileged call is 
moved out as a separate method that will throw IOException as the 
expected functionality as suggested above.

For SubSystem::getStringValue, one suggestion would be:

diff --git 
a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java 
b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
--- 
a/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
+++ 
b/src/java.base/linux/classes/jdk/internal/platform/cgroupv1/SubSystem.java
@@ -29,7 +29,11 @@
  import java.io.IOException;
  import java.math.BigInteger;
  import java.nio.file.Files;
+import java.nio.file.Path;
  import java.nio.file.Paths;
+import java.security.AccessController;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
  import java.util.ArrayList;
  import java.util.List;
  import java.util.Optional;
@@ -90,9 +94,8 @@
      public static String getStringValue(SubSystem subsystem, String 
parm) {
          if (subsystem == null) return null;

-        try(BufferedReader bufferedReader = 
Files.newBufferedReader(Paths.get(subsystem.path(), parm))) {
-            String line = bufferedReader.readLine();
-            return line;
+        try {
+            return subsystem.readStringValue(parm);
          }
          catch (IOException e) {
              return null;
@@ -100,6 +103,24 @@

      }

+    private String readStringValue(String param) throws IOException {
+        PrivilegedExceptionAction<BufferedReader> pea = () -> 
Files.newBufferedReader(Paths.get(path(), param));
+        try (BufferedReader bufferedReader = 
AccessController.doPrivileged(pea)) {
+            String line = bufferedReader.readLine();
+            return line;
+        } catch (PrivilegedActionException e) {
+            Throwable x = e.getCause();
+            if (x instanceof IOException)
+                throw (IOException)x;
+            if (x instanceof RuntimeException)
+                throw (RuntimeException)x;
+            if (x instanceof Error)
+                throw (Error)x;
+
+            throw new InternalError(x);
+        }
+    }
+