Skip to content

Fix a deadlock between WorkflowRun#save and WorkflowRun#reload#523

Merged
jglick merged 2 commits intojenkinsci:masterfrom
Vlatombe:deadlock-workflowrun-reload
Apr 15, 2025
Merged

Fix a deadlock between WorkflowRun#save and WorkflowRun#reload#523
jglick merged 2 commits intojenkinsci:masterfrom
Vlatombe:deadlock-workflowrun-reload

Conversation

@Vlatombe
Copy link
Member

@Vlatombe Vlatombe commented Apr 10, 2025

Ensure locking order

metadataGuard > WorkflowRun > CpsFlowExecution

"some-thread" id=130 (0x82) state=BLOCKED cpu=84%
    - waiting to lock <0x54046fbb> (a org.jenkinsci.plugins.workflow.job.WorkflowRun)
      owned by "Running CpsFlowExecution[Owner[some-project/1:some-project #1]]" id=27474 (0x6b52)
    at PluginClassLoader for workflow-job//org.jenkinsci.plugins.workflow.job.WorkflowRun.save(WorkflowRun.java:1252)
    at hudson.BulkChange.commit(BulkChange.java:98)
    at PluginClassLoader for workflow-cps//org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.notifyListeners(CpsFlowExecution.java:1557)
    at PluginClassLoader for workflow-cps//org.jenkinsci.plugins.workflow.cps.FlowHead.lambda$notifyNewHead$0(FlowHead.java:164)
    at PluginClassLoader for workflow-cps//org.jenkinsci.plugins.workflow.cps.FlowHead$$Lambda/0x000000080191daa0.run(Unknown Source)
    at PluginClassLoader for workflow-cps//org.jenkinsci.plugins.workflow.cps.FlowHead.notifyNewHead(FlowHead.java:169)
    at PluginClassLoader for workflow-cps//org.jenkinsci.plugins.workflow.cps.FlowHead.setNewHead(FlowHead.java:146)
    at PluginClassLoader for workflow-cps//org.jenkinsci.plugins.workflow.cps.FlowHead.setNewHead(FlowHead.java:123)
    at PluginClassLoader for workflow-cps//org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.createPlaceholderNodes(CpsFlowExecution.java:751)
    at PluginClassLoader for workflow-cps//org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.onLoad(CpsFlowExecution.java:815)
    at PluginClassLoader for workflow-job//org.jenkinsci.plugins.workflow.job.WorkflowRun.reload(WorkflowRun.java:561)
[...]

"Running CpsFlowExecution[Owner[some-project/1:some-project #1]]" id=27474 (0x6b52) state=BLOCKED cpu=64%
    - waiting to lock <0x29b5111b> (a org.jenkinsci.plugins.workflow.cps.CpsFlowExecution)
      owned by "some-thread" id=130 (0x82)
    at PluginClassLoader for workflow-cps//org.jenkinsci.plugins.workflow.cps.CpsFlowExecution$ConverterImpl.marshal(CpsFlowExecution.java:1737)
    at hudson.util.XStream2$AssociatedConverterImpl.marshal(XStream2.java:556)
    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.AbstractReferenceMarshaller$1.convertAnother(AbstractReferenceMarshaller.java:83)
    at hudson.util.RobustReflectionConverter.marshallField(RobustReflectionConverter.java:283)
    at hudson.util.RobustReflectionConverter$2.writeField(RobustReflectionConverter.java:270)
    at hudson.util.RobustReflectionConverter$2.visit(RobustReflectionConverter.java:241)
    at com.thoughtworks.xstream.converters.reflection.PureJavaReflectionProvider.visitSerializableFields(PureJavaReflectionProvider.java:174)
    at hudson.util.RobustReflectionConverter.doMarshal(RobustReflectionConverter.java:226)
    at hudson.util.RobustReflectionConverter.marshal(RobustReflectionConverter.java:163)
    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:1307)
    at com.thoughtworks.xstream.XStream.marshal(XStream.java:1296)
    at com.thoughtworks.xstream.XStream.toXML(XStream.java:1269)
    at hudson.XmlFile.write(XmlFile.java:216)
    at PluginClassLoader for workflow-support//org.jenkinsci.plugins.workflow.support.PipelineIOUtils.writeByXStream(PipelineIOUtils.java:30)
    at PluginClassLoader for workflow-job//org.jenkinsci.plugins.workflow.job.WorkflowRun.save(WorkflowRun.java:1253)
[...]

Testing done

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@Vlatombe Vlatombe requested a review from a team as a code owner April 10, 2025 14:45
@Vlatombe
Fix a deadlock between WorkflowRun#save and WorkflowRun#reload
3d080e3 
Ensure locking order

metadataGuard > WorkflowRun > CpsFlowExecution
@Vlatombe Vlatombe force-pushed the deadlock-workflowrun-reload branch from 1015892 to 3d080e3 Compare April 10, 2025 14:46
@jglick jglick added the bug label Apr 10, 2025
jglick
jglick reviewed Apr 10, 2025
View reviewed changes
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amending #472 / #510 FTR

@Vlatombe
Reorganize locks inside reload()
f1af84b 
* super.onLoad() doesn't need to lock getMetadataGuard()
* Capture fields for reading before loading CpsFlowExecution with monitor on WorkflowRun
@Vlatombe Vlatombe requested a review from jglick April 15, 2025 15:49
jglick
jglick approved these changes Apr 15, 2025
View reviewed changes
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤞

@jglick jglick merged commit 6336249 into jenkinsci:master Apr 15, 2025
17 checks passed
@Vlatombe Vlatombe deleted the deadlock-workflowrun-reload branch April 16, 2025 06:59
@felipecrs
Copy link

felipecrs commented May 30, 2025

From a user perspective, can this help with pipelines not recovering after Jenkins restart?

@jglick
Copy link
Member

jglick commented May 30, 2025

@felipecrs no, whatever problem you may be having is almost certainly unrelated.

@felipecrs
Copy link

felipecrs commented May 30, 2025

Got it, thank you @jglick.

@felipecrs
Copy link

felipecrs commented May 30, 2025

Out of curiosity, @jglick, I found out what was causing my builds not to recover after a Jenkins restart: JENKINS-75731.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jglick jglick jglick approved these changes

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Vlatombe @felipecrs @jglick