8255908: ExceptionInInitializerError due to UncheckedIOException while initializing cgroupv1 subsystem #1303

poonamparhar · 2020-11-19T01:22:01Z

Hi,

Please review this simple change that catches UncheckedIOException that can occur if /proc/self/cgroup or /proc/self/mountinfo files don't exist on the system, or if there is an interrupt while these are being read.

Testing: Tier1, Tier2 and Tier3.

Thanks,
Poonam

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8255908: ExceptionInInitializerError due to UncheckedIOException while initializing cgroupv1 subsystem

Reviewers

Aleksey Shipilev (@shipilev - Reviewer) ⚠️ Review applies to 80799f2
Severin Gehwolf (@jerboaa - Reviewer)
Bob Vandette (@bobvandette - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1303/head:pull/1303
$ git checkout pull/1303

…e initializing cgroupv1 subsystem

bridgekeeper · 2020-11-19T01:22:32Z

👋 Welcome back poonam! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2020-11-19T01:23:29Z

@poonamparhar The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2020-11-19T01:27:09Z

Webrevs

shipilev · 2020-11-19T14:36:27Z

src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1Subsystem.java

@@ -75,6 +76,8 @@ private static CgroupV1Subsystem initSubSystem() {
                 .map(line -> line.split(" "))
                 .forEach(entry -> createSubSystemController(subsystem, entry));

+        } catch (UncheckedIOException e) {


I am confused. CgroupUtil.readFilePrivileged unwraps UncheckedIOException, why this catch clause is needed? Is this because the cause might be UncheckedIOException itself? Might be easier to rewrap UncheckedIOException to just IOException in CgroupUtil.

It is there to catch the UncheckedIOException that could get thrown while processing each line and entry. Example of this exception encountered with JDK 8u.

Caused by: java.io.UncheckedIOException:
java.nio.channels.ClosedByInterruptException
at java.io.BufferedReader$1.hasNext(BufferedReader.java:574)
at java.util.Iterator.forEachRemaining(Iterator.java:115)
at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
at jdk.internal.platform.cgroupv1.Metrics.initContainerSubSystems(Metrics.java:81)
at jdk.internal.platform.cgroupv1.Metrics.(Metrics.java:51)... 41 more

Caused by: java.nio.channels.ClosedByInterruptException
at java.nio.channels.spi.AbstractInterruptibleChannel.end(AbstractInterruptibleChannel.java:202)
at sun.nio.ch.FileChannelImpl.read(FileChannelImpl.java:164)
at sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:65)
at sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:109)
at sun.nio.ch.ChannelInputStream.read(ChannelInputStream.java:103)
at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:284)
at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:326)
at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:178)
at java.io.InputStreamReader.read(InputStreamReader.java:184)
at java.io.BufferedReader.fill(BufferedReader.java:161)
at java.io.BufferedReader.readLine(BufferedReader.java:324)
at java.io.BufferedReader.readLine(BufferedReader.java:389)
at java.io.BufferedReader$1.hasNext(BufferedReader.java:571)

Got it, thanks.

shipilev · 2020-11-19T14:36:53Z

src/java.base/linux/classes/jdk/internal/platform/CgroupUtil.java

+        } catch (UncheckedIOException e) {
+            throw e.getCause();


There are other reader methods in the same file, should this block be added to them as well?

This change takes care of the case when reading of files /proc/self/cgroup or /proc/self/mountinfo might fail due to some reason that can cause the initialization of cgroupv1 subsystem (initSubSystem) to fail. Looking at the usages of other reader methods, I think it makes sense to catch UncheckedIOException there as well.

Updated the code changes, and added the catch blocks for the rest of the reader methods and also for the cgroupv2 subsystem.

…lso for the rest of reader methods in CgroupUtil.java

shipilev

This looks good to me. @jerboaa might want to take a look.

shipilev · 2020-11-19T17:44:40Z

src/java.base/linux/classes/jdk/internal/platform/cgroupv1/CgroupV1Subsystem.java

@@ -75,6 +76,8 @@ private static CgroupV1Subsystem initSubSystem() {
                 .map(line -> line.split(" "))
                 .forEach(entry -> createSubSystemController(subsystem, entry));

+        } catch (UncheckedIOException e) {


Got it, thanks.

openjdk · 2020-11-19T17:58:11Z

@poonamparhar This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8255908: ExceptionInInitializerError due to UncheckedIOException while initializing cgroupv1 subsystem

Reviewed-by: shade, sgehwolf, bobv

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 53 new commits pushed to the master branch:

cc0ed40: 8037384: Fix wording in Javadoc of java.io.Serializable
19b2898: 8256751: Incremental rebuild with precompiled header fails when touching a header file
4dd71ae: 8256803: ProblemList runtime/ReservedStack/ReservedStackTestCompiler.java on linux-aarch64
2c3a2be: 8211449: Correction to the spec of implicit negative subpattern in DecimalFormat
11bfdc5: 8235304: JPackage Windows test should be added to set Publisher
2ae3e51: 8229845: Decrease memory consumption of BigInteger.toString()
ff00c59: 8256569: Add C2 compiler stress flags to CTW
e7c7469: 8246378: [Windows] assert on MethodHandle logging code
98a5d5a: 8256664: Shenandoah: Cleanup after JDK-8212879
b99fd4c: 8033441: print line numbers with -XX:+PrintOptoAssembly
... and 43 more: https://git.openjdk.java.net/jdk/compare/33d3918e5aeab4a5cc223476e15d0201e99d61d7...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

bobvandette · 2020-11-19T19:29:25Z

Shouldn't you add a check in src/java.base/linux/classes/jdk/internal/platform/cgroupv2/CgroupV2Subsystem.java:sumTokensIOStat ?

What about getLongEntry in src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemController.java? There are streams operations occuring there as well.

…ngEntry methods

poonamparhar · 2020-11-19T20:49:07Z

@bobvandette sumTokensIOStat() and getLongEntry() should also have the catch blocks, and I have added those. Please review the latest commit.

jerboaa · 2020-11-20T09:10:05Z

@poonamparhar Wouldn't we be able to reproduce this by a test? E.g. test/jdk/jdk/internal/platform/cgroup/TestCgroupSubsystemFactory.java It should already be checking some of this with mountInfoFileNotFound() and cgroupsFileNotFound(). Thoughts?

jerboaa

This seems OK. Ideally, I'd like for this to be tested. If it's interrupts it's going to be difficult. Missing files should be easy enough to test.

Aside: We really need to streamline file reading. We have too many of them unnecessarily all over the place. JDK-8254001 should fix that. I'll prioritize it.

poonamparhar · 2020-11-20T15:10:37Z

@jerboaa Thanks for the review. As for the missing files, looks like mountInfoFileNotFound() and cgroupsFileNotFound() should be testing it.

This issue is being faced by a customer and they encountered failures due to interrupts during reading. These changes have been verified by them.

jerboaa · 2020-11-20T15:50:40Z

@jerboaa Thanks for the review. As for the missing files, looks like mountInfoFileNotFound() and cgroupsFileNotFound() should be testing it.

This issue is being faced by a customer and they encountered failures due to interrupts during reading. These changes have been verified by them.

Thanks. That's OK. Preference would be to have an automated test for this issue, but this seems difficult in this case.

poonamparhar · 2020-11-20T18:38:59Z

/integrate

openjdk · 2020-11-20T18:40:33Z

@poonamparhar Since your change was applied there have been 54 commits pushed to the master branch:

5ad1e22: 8256658: Shenandoah: Deadlock between nmethod_entry_barrier and concurrent code root evacuator
cc0ed40: 8037384: Fix wording in Javadoc of java.io.Serializable
19b2898: 8256751: Incremental rebuild with precompiled header fails when touching a header file
4dd71ae: 8256803: ProblemList runtime/ReservedStack/ReservedStackTestCompiler.java on linux-aarch64
2c3a2be: 8211449: Correction to the spec of implicit negative subpattern in DecimalFormat
11bfdc5: 8235304: JPackage Windows test should be added to set Publisher
2ae3e51: 8229845: Decrease memory consumption of BigInteger.toString()
ff00c59: 8256569: Add C2 compiler stress flags to CTW
e7c7469: 8246378: [Windows] assert on MethodHandle logging code
98a5d5a: 8256664: Shenandoah: Cleanup after JDK-8212879
... and 44 more: https://git.openjdk.java.net/jdk/compare/33d3918e5aeab4a5cc223476e15d0201e99d61d7...master

Your commit was automatically rebased without conflicts.

Pushed as commit 8d9cf48.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

…e initializing cgroupv1 subsystem Reviewed-by: shade, sgehwolf, bobv

openjdk bot added the rfr label Nov 19, 2020

openjdk bot added the core-libs label Nov 19, 2020

shipilev suggested changes Nov 19, 2020

View reviewed changes

Added UncheckedIOException catch blocks for cgroupv2 subsystem, and a…

80799f2

…lso for the rest of reader methods in CgroupUtil.java

shipilev approved these changes Nov 19, 2020

View reviewed changes

openjdk bot added the ready label Nov 19, 2020

Added UnchedkedIOException catch blocks for sumTokensIOStat and getLo…

8e39e8f

…ngEntry methods

jerboaa reviewed Nov 20, 2020

View reviewed changes

jerboaa approved these changes Nov 20, 2020

View reviewed changes

bobvandette approved these changes Nov 20, 2020

View reviewed changes

openjdk bot closed this Nov 20, 2020

openjdk bot added integrated and removed ready rfr labels Nov 20, 2020

openjdk-notifier bot referenced this pull request Nov 20, 2020

8255908: ExceptionInInitializerError due to UncheckedIOException whil…

8d9cf48

…e initializing cgroupv1 subsystem Reviewed-by: shade, sgehwolf, bobv

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8255908: ExceptionInInitializerError due to UncheckedIOException while initializing cgroupv1 subsystem #1303

8255908: ExceptionInInitializerError due to UncheckedIOException while initializing cgroupv1 subsystem #1303

poonamparhar commented Nov 19, 2020 •

edited by openjdk bot

Loading

bridgekeeper bot commented Nov 19, 2020

openjdk bot commented Nov 19, 2020

mlbridge bot commented Nov 19, 2020 •

edited

Loading

shipilev Nov 19, 2020

poonamparhar Nov 19, 2020

shipilev Nov 19, 2020

shipilev Nov 19, 2020

poonamparhar Nov 19, 2020

poonamparhar Nov 19, 2020

shipilev left a comment

shipilev Nov 19, 2020

openjdk bot commented Nov 19, 2020 •

edited

Loading

bobvandette commented Nov 19, 2020

poonamparhar commented Nov 19, 2020

jerboaa commented Nov 20, 2020

jerboaa left a comment

poonamparhar commented Nov 20, 2020

jerboaa commented Nov 20, 2020

poonamparhar commented Nov 20, 2020

openjdk bot commented Nov 20, 2020

8255908: ExceptionInInitializerError due to UncheckedIOException while initializing cgroupv1 subsystem #1303

8255908: ExceptionInInitializerError due to UncheckedIOException while initializing cgroupv1 subsystem #1303

Conversation

poonamparhar commented Nov 19, 2020 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Nov 19, 2020

openjdk bot commented Nov 19, 2020

mlbridge bot commented Nov 19, 2020 • edited Loading

Webrevs

shipilev Nov 19, 2020

Choose a reason for hiding this comment

poonamparhar Nov 19, 2020

Choose a reason for hiding this comment

shipilev Nov 19, 2020

Choose a reason for hiding this comment

shipilev Nov 19, 2020

Choose a reason for hiding this comment

poonamparhar Nov 19, 2020

Choose a reason for hiding this comment

poonamparhar Nov 19, 2020

Choose a reason for hiding this comment

shipilev left a comment

Choose a reason for hiding this comment

shipilev Nov 19, 2020

Choose a reason for hiding this comment

openjdk bot commented Nov 19, 2020 • edited Loading

bobvandette commented Nov 19, 2020

poonamparhar commented Nov 19, 2020

jerboaa commented Nov 20, 2020

jerboaa left a comment

Choose a reason for hiding this comment

poonamparhar commented Nov 20, 2020

jerboaa commented Nov 20, 2020

poonamparhar commented Nov 20, 2020

openjdk bot commented Nov 20, 2020

poonamparhar commented Nov 19, 2020 •

edited by openjdk bot

Loading

mlbridge bot commented Nov 19, 2020 •

edited

Loading

openjdk bot commented Nov 19, 2020 •

edited

Loading