Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-68071

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

    • 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)
      

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

          Jesse Glick added a comment -

          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 - 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 -

          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 - 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.

          Devin Nusbaum added a comment -

          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 - 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 - - 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 - - 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 - - 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 - - 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.

          Jesse Glick added a comment -

          I agree that it would be preferable to avoid serializing AtomicBoolean here. May be more “code” than adding another exclusion to core, but worth cleaning up this tech debt anyway.

          Jesse Glick added a comment - I agree that it would be preferable to avoid serializing AtomicBoolean here. May be more “code” than adding another exclusion to core, but worth cleaning up this tech debt anyway.

          Jesse Glick added a comment -

          Starting by dealing with the backlog of dependency updates in this plugin (CC carroll).

          Jesse Glick added a comment - Starting by dealing with the backlog of dependency updates in this plugin (CC carroll ).

          Jesse Glick added a comment -

          Jesse Glick added a comment - I am relieved to report that the proposed transient patch causes https://github.com/jenkinsci/workflow-cps-plugin/blob/765d763c170f54c080a879539255c87d0f636eb2/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionTest.java#L203 to fail as expected.

          Basil Crow added a comment -

          Thank you!

          Basil Crow added a comment - Thank you!

          Basil Crow added a comment -

          Verified in an end-to-end test on Jenkins 2.339 and Java 17 that going to Thread Dump, Serialized program state in the GUI now works.

          Basil Crow added a comment - Verified in an end-to-end test on Jenkins 2.339 and Java 17 that going to Thread Dump, Serialized program state in the GUI now works.

            jglick Jesse Glick
            basil Basil Crow
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: