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);
+ }
+ }
+
