[JENKINS-68071] Thread Dump, Serialized program state does not work on Java 17

Type: Bug
Resolution: Fixed
Priority: Minor
Component/s: workflow-cps-plugin
Labels:
None

Similar Issues:
Powered by SuggestiMate

Show
Epic Link:
Java 17 Phase 1: Support Java 17
Released As:
2680.vf642ed4fa_d55

Steps to reproduce

With Java 17, run:

mvn clean verify -Dtest=org.jenkinsci.plugins.workflow.cps.CpsBodyExecutionTest#closureCapturesCpsBodyExecution,org.jenkinsci.plugins.workflow.cps.CpsBodyExecutionTest#popContextVarsOnBodyCompletion,org.jenkinsci.plugins.workflow.cps.CpsThreadDumpActionTest#doProgramDotXml -Djenkins.version=2.339 -Djenkins-test-harness.version=1721.v385389722736 '-DargLine=--add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.util.concurrent=ALL-UNNAMED'

Expected results

The tests pass.

Actual results

The tests fail with:

[ERROR] org.jenkinsci.plugins.workflow.cps.CpsBodyExecutionTest.closureCapturesCpsBodyExecution  Time elapsed: 7.439 s  <<< ERROR!
com.thoughtworks.xstream.converters.ConversionException: 
No converter available
---- Debugging information ----
message             : No converter available
type                : java.util.concurrent.atomic.AtomicBoolean
converter           : com.thoughtworks.xstream.converters.reflection.ReflectionConverter
message[1]          : Unable to make field private static final long java.util.concurrent.atomic.AtomicBoolean.serialVersionUID accessible: module java.base does not "opens java.util.concurrent.atomic" to unnamed module @6b09bb57
-------------------------------
        at com.thoughtworks.xstream.core.DefaultConverterLookup.lookupConverterForType(DefaultConverterLookup.java:88)
        at com.thoughtworks.xstream.XStream$1.lookupConverterForType(XStream.java:478)
        at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:49)
        at com.thoughtworks.xstream.core.AbstractReferenceMarshaller$1.convertAnother(AbstractReferenceMarshaller.java:83)
        at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshallField(AbstractReflectionConverter.java:270)
        at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter$2.writeField(AbstractReflectionConverter.java:174)
        at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.doMarshal(AbstractReflectionConverter.java:262)
        at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.marshal(AbstractReflectionConverter.java:90)
        at com.thoughtworks.xstream.core.AbstractReferenceMarshaller.convert(AbstractReferenceMarshaller.java:68)
        at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:59)
        at com.thoughtworks.xstream.core.TreeMarshaller.convertAnother(TreeMarshaller.java:44)
        at com.thoughtworks.xstream.core.TreeMarshaller.start(TreeMarshaller.java:83)
        at com.thoughtworks.xstream.core.AbstractTreeMarshallingStrategy.marshal(AbstractTreeMarshallingStrategy.java:37)
        at com.thoughtworks.xstream.XStream.marshal(XStream.java:1266)
        at com.thoughtworks.xstream.XStream.marshal(XStream.java:1255)
        at com.thoughtworks.xstream.XStream.toXML(XStream.java:1228)
        at com.thoughtworks.xstream.XStream.toXML(XStream.java:1215)
        at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup.asXml(CpsThreadGroup.java:606)
        at org.jenkinsci.plugins.workflow.cps.CpsBodyExecutionTest.lambda$closureCapturesCpsBodyExecution$4(CpsBodyExecutionTest.java:239)
        at org.jvnet.hudson.test.RestartableJenkinsRule$3.evaluate(RestartableJenkinsRule.java:243)
        at org.jvnet.hudson.test.RestartableJenkinsRule$6.evaluate(RestartableJenkinsRule.java:291)
        at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:606)
        at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:299)
        at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:293)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.lang.Thread.run(Thread.java:833)

is caused by

JENKINS-25736 Ability to pause a running flow

Resolved

links to

workflow-cps-plugin #515

Basil Crow created issue - 2022-03-17 19:33

Basil Crow added a comment - 2022-03-17 19:40

Pinging carroll, dnusbaum, and jglick in case they are interested in taking a quick glance at this for initial triage purposes. I am mostly interested in determining if this is an issue in test code or in the code under test (i.e. a production issue) and if any obvious solution comes to mind at first blush. No pressure to get involved with this, and please do not feel obligated to spend a lot of time on this, but if you are interested, anything that comes to mind offhand would be appreciated before I begin evaluating this ticket more closely.

Basil Crow added a comment - 2022-03-17 19:40 Pinging carroll , dnusbaum , and jglick in case they are interested in taking a quick glance at this for initial triage purposes. I am mostly interested in determining if this is an issue in test code or in the code under test (i.e. a production issue) and if any obvious solution comes to mind at first blush. No pressure to get involved with this, and please do not feel obligated to spend a lot of time on this, but if you are interested, anything that comes to mind offhand would be appreciated before I begin evaluating this ticket more closely.

Basil Crow added a comment - 2022-03-17 19:46

FYI this gets the tests to pass, but I'm not sure if it is the right fix or not.

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java
index 0022dead..a9e8f580 100644
--- a/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java
+++ b/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java
@@ -145,7 +145,7 @@ public final class CpsThreadGroup implements Serializable {
      * on hold (for example while inspecting a suspicious state or to perform a maintenance
      * when a failure is predictable.)
      */
-    private /*almost final*/ AtomicBoolean paused = new AtomicBoolean();
+    private /*almost final*/ transient AtomicBoolean paused = new AtomicBoolean();
 
     /**
      * "Exported" closures that are referenced by live {@link CpsStepContext}s.

Basil Crow added a comment - 2022-03-17 19:46 FYI this gets the tests to pass, but I'm not sure if it is the right fix or not. diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java index 0022dead..a9e8f580 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThreadGroup.java @@ -145,7 +145,7 @@ public final class CpsThreadGroup implements Serializable { * on hold (for example while inspecting a suspicious state or to perform a maintenance * when a failure is predictable.) */ - private /*almost final*/ AtomicBoolean paused = new AtomicBoolean(); + private /*almost final*/ transient AtomicBoolean paused = new AtomicBoolean(); /** * "Exported" closures that are referenced by live {@link CpsStepContext}s.

Jesse Glick added a comment - 2022-03-17 20:07

No that fix is not correct; paused is deliberately serialized. We could possibly switch the serialized field to be a boolean with a field rename but it would be trickier than that.

CpsThreadGroup.asXml is rarely called outside tests, but there is a GUI gesture, so it would be best not to break it. Not sure if this regression in XStream is intentional.

Jesse Glick added a comment - 2022-03-17 20:07 No that fix is not correct; paused is deliberately serialized. We could possibly switch the serialized field to be a boolean with a field rename but it would be trickier than that. CpsThreadGroup.asXml is rarely called outside tests, but there is a GUI gesture, so it would be best not to break it. Not sure if this regression in XStream is intentional.

Basil Crow added a comment - 2022-03-17 20:15

Thanks for the tip. I was able to reproduce this in the GUI by going to Thread Dump, Serialized program state. So it is technically a production bug – the first one I've found after sifting through dozens of PCT failures! I feel vindicated.

Basil Crow added a comment - 2022-03-17 20:15 Thanks for the tip. I was able to reproduce this in the GUI by going to Thread Dump , Serialized program state . So it is technically a production bug – the first one I've found after sifting through dozens of PCT failures! I feel vindicated.

Basil Crow made changes - 2022-03-17 20:15

Summary

Original: Pipeline: Groovy tests fail on Java 17

New: Thread Dump, Serialized program state does not work on Java 17

Basil Crow made changes - 2022-03-17 20:16

Priority

Original: Major [ 3 ]

New: Minor [ 4 ]

Devin Nusbaum added a comment - 2022-03-17 20:16

My guess is that the proximate cause here is the changes to JDK encapsulation in JDK 16/17 (JEP 403). In JDK 9-15, the default mode was illegal-access=permit, which just logs a warning. In JDK 16, it became illegal-access=deny. In JDK 17, illegal-access is ignored, and you have to use add-opens or similar options for specific packages instead. From a quick search, I did not see anything about this kind of issue in XStream's issue tracker.

Devin Nusbaum added a comment - 2022-03-17 20:16 My guess is that the proximate cause here is the changes to JDK encapsulation in JDK 16/17 ( JEP 403 ). In JDK 9-15, the default mode was illegal-access=permit , which just logs a warning. In JDK 16, it became illegal-access=deny . In JDK 17, illegal-access is ignored, and you have to use add-opens or similar options for specific packages instead. From a quick search, I did not see anything about this kind of issue in XStream's issue tracker.

Devin Nusbaum added a comment - 2022-03-17 20:24 - edited

Hmm, yeah looking at https://github.com/x-stream/xstream/commit/b9960d46f230bf6fc70c3f1005d5341de9faff2a, the XStream maintainers added a bunch of --add-opens clauses when running their tests using JDK 17, so I guess they might expect users to do the same if they want to serialize JDK classes.

Devin Nusbaum added a comment - 2022-03-17 20:24 - edited Hmm, yeah looking at https://github.com/x-stream/xstream/commit/b9960d46f230bf6fc70c3f1005d5341de9faff2a , the XStream maintainers added a bunch of --add-opens clauses when running their tests using JDK 17, so I guess they might expect users to do the same if they want to serialize JDK classes.

Basil Crow added a comment - 2022-03-17 20:24 - edited

dnusbaum Absolutely right, and the upstream XStream issue is x-stream/xstream#262. We are already shipping the WAR with <Add-Opens>java.base/java.lang java.base/java.io java.base/java.util java.base/java.util.concurrent</Add-Opens> in MANIFEST.MF, which is needed to get XStream off the ground at all. With those directives, I have been able to run all core tests and all BOM tests without reflection errors, except for this one. While we could add an add-opens directive for java.base/java.util.concurrent.atomic just for this case, I think it would be preferable to avoid it if we can. Of course, there might be some other case that requires us to add an add-opens for java.base/java.util.concurrent.atomic, but so far I haven't found any besides this one.

Basil Crow added a comment - 2022-03-17 20:24 - edited dnusbaum Absolutely right, and the upstream XStream issue is x-stream/xstream#262 . We are already shipping the WAR with <Add-Opens>java.base/java.lang java.base/java.io java.base/java.util java.base/java.util.concurrent</Add-Opens> in MANIFEST.MF , which is needed to get XStream off the ground at all. With those directives, I have been able to run all core tests and all BOM tests without reflection errors, except for this one. While we could add an add-opens directive for java.base/java.util.concurrent.atomic just for this case, I think it would be preferable to avoid it if we can. Of course, there might be some other case that requires us to add an add-opens for java.base/java.util.concurrent.atomic , but so far I haven't found any besides this one.

Assignee:: Jesse Glick

Reporter:: Basil Crow

Votes:: 0 Vote for this issue

Watchers:: 3 Start watching this issue

Created:: 2022-03-17 19:33

Updated:: 2022-03-18 21:22

Resolved:: 2022-03-18 18:27

Jenkins

Details

Description

Steps to reproduce

Expected results

Actual results

Attachments

Issue Links

Activity

Collapse comment: Basil Crow added a comment - 2022-03-17 19:40

Expand comment: Basil Crow added a comment - 2022-03-17 19:40

Collapse comment: Basil Crow added a comment - 2022-03-17 19:46

Expand comment: Basil Crow added a comment - 2022-03-17 19:46

Collapse comment: Jesse Glick added a comment - 2022-03-17 20:07

Expand comment: Jesse Glick added a comment - 2022-03-17 20:07

Collapse comment: Basil Crow added a comment - 2022-03-17 20:15

Expand comment: Basil Crow added a comment - 2022-03-17 20:15

Collapse comment: Devin Nusbaum added a comment - 2022-03-17 20:16

Expand comment: Devin Nusbaum added a comment - 2022-03-17 20:16

Collapse comment: Devin Nusbaum added a comment - 2022-03-17 20:24, Edited by Devin Nusbaum - 2022-03-17 20:24

Expand comment: Devin Nusbaum added a comment - 2022-03-17 20:24, Edited by Devin Nusbaum - 2022-03-17 20:24

Collapse comment: Basil Crow added a comment - 2022-03-17 20:24, Edited by Basil Crow - 2022-03-17 20:24

Expand comment: Basil Crow added a comment - 2022-03-17 20:24, Edited by Basil Crow - 2022-03-17 20:24

People

Dates