search

jenkinsci / ghprb-plugin

github pull requests builder plugin for Jenkins
https://plugins.jenkins.io/ghprb/
MIT License
507 stars 612 forks source link

GhprbCancelBuildsOnUpdate loops through queue items #741

Closed mayt closed 5 years ago

mayt commented 5 years ago

The GhprbCancelBuildsOnUpdate's cancelCurrentBuilds does not loop through the project's queue items which will cause an infinite loop when new scheduled builds are added. In certain cases the newly scheduled jobs using Ghprb will get caught in the loop and will deadlock the executor and worker threads. See the stacktrace below. 

Executor #0 for <Redacted>: executing job #284
"Executor #0 for <Redacted> : executing job #284" Id=46759 Group=main BLOCKED on org.jenkinsci.plugins.ghprb.GhprbPullRequest@71a5579b owned by "Thread-3400" Id=45802
    at org.jenkinsci.plugins.ghprb.GhprbPullRequest.setPullRequest(GhprbPullRequest.java:836)
    -  blocked on org.jenkinsci.plugins.ghprb.GhprbPullRequest@71a5579b
    at org.jenkinsci.plugins.ghprb.GhprbPullRequest.getPullRequest(GhprbPullRequest.java:827)
    at org.jenkinsci.plugins.ghprb.GhprbBuilds.onStarted(GhprbBuilds.java:112)
    at org.jenkinsci.plugins.ghprb.GhprbBuildListener.onStarted(GhprbBuildListener.java:20)
    at hudson.model.listeners.RunListener.fireStarted(RunListener.java:240)
    at hudson.model.Run.execute(Run.java:1794)
    at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
    at hudson.model.ResourceController.execute(ResourceController.java:97)
    at hudson.model.Executor.run(Executor.java:429)

Thread-3400
"Thread-3400" Id=45802 Group=main RUNNABLE
    at java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1042)
    at java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1042)
    at org.jenkinsci.plugins.ghprb.extensions.build.GhprbCancelBuildsOnUpdate.cancelCurrentBuilds(GhprbCancelBuildsOnUpdate.java:60)
    at org.jenkinsci.plugins.ghprb.extensions.build.GhprbCancelBuildsOnUpdate.onScheduleBuild(GhprbCancelBuildsOnUpdate.java:114)
    at org.jenkinsci.plugins.ghprb.GhprbTrigger.scheduleBuild(GhprbTrigger.java:379)
    at org.jenkinsci.plugins.ghprb.GhprbBuilds.build(GhprbBuilds.java:94)
    at org.jenkinsci.plugins.ghprb.GhprbPullRequest.build(GhprbPullRequest.java:555)
    at org.jenkinsci.plugins.ghprb.GhprbPullRequest.tryBuild(GhprbPullRequest.java:548)
    -  locked org.jenkinsci.plugins.ghprb.GhprbPullRequest@71a5579b
    at org.jenkinsci.plugins.ghprb.GhprbPullRequest.check(GhprbPullRequest.java:226)
    at org.jenkinsci.plugins.ghprb.GhprbRepository.onPullRequestHook(GhprbRepository.java:394)
    at org.jenkinsci.plugins.ghprb.GhprbTrigger.handlePR(GhprbTrigger.java:746)
    at org.jenkinsci.plugins.ghprb.GhprbRootAction$2.run(GhprbRootAction.java:257)

Thread-3476
"Thread-3476" Id=46485 Group=main BLOCKED on org.jenkinsci.plugins.ghprb.GhprbPullRequest@71a5579b owned by "Thread-3400" Id=45802
    at org.jenkinsci.plugins.ghprb.GhprbPullRequest.updatePR(GhprbPullRequest.java:346)
    -  blocked on org.jenkinsci.plugins.ghprb.GhprbPullRequest@71a5579b
    at org.jenkinsci.plugins.ghprb.GhprbPullRequest.check(GhprbPullRequest.java:221)
    at org.jenkinsci.plugins.ghprb.GhprbRepository.onPullRequestHook(GhprbRepository.java:394)
    at org.jenkinsci.plugins.ghprb.GhprbTrigger.handlePR(GhprbTrigger.java:746)
    at org.jenkinsci.plugins.ghprb.GhprbRootAction$2.run(GhprbRootAction.java:257)

Thread-3533
"Thread-3533" Id=47105 Group=main BLOCKED on org.jenkinsci.plugins.ghprb.GhprbPullRequest@71a5579b owned by "Thread-3400" Id=45802
    at org.jenkinsci.plugins.ghprb.GhprbPullRequest.updatePR(GhprbPullRequest.java:346)
    -  blocked on org.jenkinsci.plugins.ghprb.GhprbPullRequest@71a5579b
    at org.jenkinsci.plugins.ghprb.GhprbPullRequest.check(GhprbPullRequest.java:221)
    at org.jenkinsci.plugins.ghprb.GhprbRepository.onPullRequestHook(GhprbRepository.java:394)
    at org.jenkinsci.plugins.ghprb.GhprbTrigger.handlePR(GhprbTrigger.java:746)
    at org.jenkinsci.plugins.ghprb.GhprbRootAction$2.run(GhprbRootAction.java:257)

This patches the looping logic to iterate through all the items and exits the loop.

bjoernhaeuser commented 5 years ago

@mayt the tests werent successful, could you check please?

mayt commented 5 years ago

Retest this please

mayt commented 5 years ago

@bjoernhaeuser looks like an unrelated test failed. Would you be able to rerun the test?

mayt commented 5 years ago

@bjoernhaeuser can you take another look?

bjoernhaeuser commented 5 years ago

Thank you!

nik9000 commented 5 years ago

@bjoernhaeuser is there any chance of getting another release of this plugin? I discovered 82 threads spinning on the loop that this PR removes this morning, leaving the master's load average at about 90.