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

[JENKINS-62545] Attempt to detect cycles when iterating through siblings in FlowGraphTable #108

Merged
merged 3 commits into from
Jun 5, 2020

Conversation

dwnusbaum
Copy link
Member

@dwnusbaum dwnusbaum commented May 27, 2020

See JENKINS-62545.

I saw a report recently of a thread stuck inside of the while loop in FlowGraphTable.Row.addTreeSibling. I am not sure how this could be happening, and I do not have any reproduction steps, but the only thing that comes to mind is an infinite loop caused by a cycle in the graph (probably not the actual flow graph, just in the representation in FlowGraphTable).

Here is a draft PR that tries to detect cycles so that we can understand what the real bug is. I have not tested it yet, and I am not sure if this code is covered by unit tests or not, so it needs to be verified manually.

CC @res0nance

EDIT: I got more info, so now I know the proximate cause of the issue, although I am still not sure about the root cause. It turns out that the actual flow graph itself was corrupted. Here is a simplified form of its structure, the sh step is the problematic node:

               b1Start-...-b1End
              /                 \
start-parStart-b2Start-...-b2End-parEnd-...-end
              \                 /          /
               b3Start-...----sh--...-b3End

The problem is that the sh step is the parent of the end of a parallel step, but somehow also the parent of additional steps. FlowGraphTable doesn't expect the graph to have a structure like this so things break. The build was manually aborted while the parallel branches were executing, yet somehow branch 3 continued executing despite the end node for the parallel step being linked to one of the nodes in that branch. I don't have access to Jenkins system logs from when the build was executed to know if there were any warnings. The Pipeline in question looks relatively normal (there aren't nested parallels or anything like that).

I am not sure how this could happen, and I wasn't able to reproduce the issue from scratch.

I think it's reasonable to make FlowGraphTable throw an exception for a corrupted graph like this to avoid infinite loops.

@res0nance
Copy link

Hey Devin 👋 , thanks for looking into this. I've a build that thankfully still exhibits this issue. Since it might contain sensitive data, I'll reach out to you out of band on this.

@dwnusbaum
Copy link
Member Author

CC @jglick have you ever seen a flow graph with the structure in the description of this PR? Any idea of what could cause it?

@jglick
Copy link
Member

jglick commented Jun 1, 2020

No idea what the root cause could be, and I agree that throwing something like an IllegalStateException in these circumstances would be appropriate.

@dwnusbaum dwnusbaum changed the title Attempt to detect cycles when iterating through siblings in FlowGraphTable [JENKINS-62545] Attempt to detect cycles when iterating through siblings in FlowGraphTable Jun 3, 2020
@dwnusbaum
Copy link
Member Author

There was no test coverage for FlowGraphTable in this plugin, so I added some smoke tests and a regression test for JENKINS-62545.

@dwnusbaum dwnusbaum marked this pull request as ready for review June 3, 2020 21:01
@dwnusbaum dwnusbaum merged commit 177f531 into jenkinsci:master Jun 5, 2020
@dwnusbaum dwnusbaum deleted the flow-graph-loop branch June 5, 2020 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants