Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class loading deadlock between PermalinkProjectAction.Permalink & PeepholePermalink #8736

Merged
merged 7 commits into from Dec 9, 2023

Conversation

jglick
Copy link
Member

@jglick jglick commented Dec 1, 2023

A functional test in CloudBees CI was observed to hang occasionally. Inspecting the thread dump revealed an apparent class loading deadlock. It is unknown whether this would affect production scenarios.

Fundamentally the problem is the use of static final fields for non-constant expressions. It would be incompatible to just delete these fields, unfortunately; plugins using them include for example

  • build-symlink
  • build-alias-setter
  • copyartifact
  • run-selector
  • infradna-backup (CloudBees CI)

As noted here it is not always clear how to solve these problems compatibly.

Testing done

Test usually deadlocks. Thread dump after 7dee8b2

"main" #1 prio=5 os_prio=0 cpu=350.37ms elapsed=13.59s tid=0x00007f9848016000 nid=0x958f in Object.wait()  [0x00007f984f0b3000]
   java.lang.Thread.State: RUNNABLE
	at jenkins.model.PeepholePermalinkTest.classLoadingDeadlock(PeepholePermalinkTest.java:42)
	at …

"Thread-1" #16 prio=5 os_prio=0 cpu=0.99ms elapsed=13.17s tid=0x00007f9848b62800 nid=0x95ab in Object.wait()  [0x00007f981c8f4000]
   java.lang.Thread.State: RUNNABLE
	at hudson.model.PermalinkProjectAction$Permalink.<clinit>(PermalinkProjectAction.java:114)
	at jenkins.model.PeepholePermalinkTest.lambda$classLoadingDeadlock$0(PeepholePermalinkTest.java:39)
	at jenkins.model.PeepholePermalinkTest$$Lambda$328/0x000000084010d040.run(Unknown Source)
	at java.lang.Thread.run(java.base@11.0.20.1/Thread.java:829)

Proposed changelog entries

  • Preventing a deadlock that can occur when loading PermalinkProjectAction.Permalink.

Before the changes are marked as ready-for-merge:

Maintainer checklist

Edit tasklist title
Beta Give feedback Tasklist Maintainer checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. There are at least two (2) approvals for the pull request and no outstanding requests for change.
    Options
  2. Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
    Options
  3. Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
    Options
  4. Proper changelog labels are set so that the changelog can be generated automatically.
    Options
  5. If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
    Options
  6. If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).
    Options

@jglick

This comment was marked as outdated.

@jglick

This comment was marked as outdated.

@jglick jglick changed the title Class loading deadlock between PermalinkProjectAction & PeepholePermalink Class loading deadlock between PermalinkProjectAction.Permalink & PeepholePermalink Dec 1, 2023
@jglick jglick marked this pull request as ready for review December 1, 2023 19:39
@jglick jglick requested a review from Vlatombe December 1, 2023 19:39
@NotMyFault NotMyFault added the bug For changelog: Minor bug. Will be listed after features label Dec 7, 2023
@NotMyFault NotMyFault requested a review from a team December 7, 2023 21:24
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

/label ready-for-merge


This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Dec 8, 2023
@NotMyFault NotMyFault merged commit f9a777b into jenkinsci:master Dec 9, 2023
17 checks passed
@jglick jglick deleted the PeepholePermalinkTest branch December 11, 2023 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For changelog: Minor bug. Will be listed after features ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
4 participants